將 AI 審查結果集中到單一 Pull Review #27
@@ -1,16 +1,9 @@
|
||||
[
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Mage",
|
||||
"location": "app/config.test.js",
|
||||
"suggestion": "`shouldSkipOpenCodeTLSVerify` 函式的新增測試案例未能涵蓋所有可能的輸入情境。在 `process.env.OPENCODE_SKIP_TLS_VERIFY !== 'false'` 的新邏輯下,應增加測試案例來驗證當環境變數設定為空字串 `''`、字串 `'0'` 或其他任意非 `'false'` 字串時,函式是否如預期般返回 `true`(跳過 TLS 驗證)。這有助於確保此關鍵安全邏輯的行為符合預期,並揭示潛在的誤配置風險。",
|
||||
"is_new": true
|
||||
},
|
||||
{
|
||||
"level": "warning",
|
||||
"role": "Maya",
|
||||
"location": "app/preflight.test.js",
|
||||
"suggestion": "在 `preflight.test.js` 中,關於 `httpsAgent` 的測試案例也已涵蓋了預設行為(跳過 TLS)和明確設定為 `false`(不跳過 TLS)的情況。請新增一個測試,驗證當環境變數 `process.env.OPENCODE_SKIP_TLS_VERIFY` 明確設定為 `'true'` 時,`verifyLLM` 函式是否會傳遞一個不安全的 `httpsAgent` 給 OpenCode 服務進行預檢。",
|
||||
"level": "critical",
|
||||
"role": "Assassin",
|
||||
"location": "app/preflight.test.js:201",
|
||||
"suggestion": "此測試進一步確認了 `OPENCODE_SKIP_TLS_VERIFY = 'true'` 會導致 HTTPS 代理設定為 `rejectUnauthorized: false`,即跳過 TLS 憑證驗證。這是一個嚴重的「不安全預設」或「不安全配置」問題。\n\n雖然這是一個明確的設定,但跳過 TLS 驗證會使應用程式容易受到中間人(MITM)攻擊。攻擊者可以在應用程式與 OpenCode 服務之間偽造伺服器身份,攔截、竊聽或篡改所有通訊內容,進而竊取機密資訊或注入惡意指令。\n\n建議除非在極端受控的環境下,否則應避免跳過 TLS 驗證。如果確實需要此功能,應確保其使用受到嚴格的審查和限制,並在文件上明確標示其安全風險。更安全的做法是配置正確的憑證信任鏈,而不是禁用驗證。",
|
||||
"is_new": true
|
||||
}
|
||||
]
|
||||
|
||||
@@ -10,13 +10,13 @@
|
||||
- 若有提供 `GITEA_COMMENT_TOKEN`,額外用它驗證可用(呼叫 `GET /api/v1/user`),確保後續發 comment 不會因 token 失效而中斷
|
||||
- git push 認證可用:用與第 8 點 commit/push 完全相同的 askpass + remote URL 機制跑一次唯讀的 `git ls-remote`,提前抓出 askpass 無法執行或 HTTP 認證失敗(例如 `could not read Username`)的問題。此路徑與上面的 REST API 不同,API token 有效不代表 git push 一定能用,故獨立驗證
|
||||
- 已選定一個 LLM provider,且其 API Key 至少有一把通過驗證:實際送出一個最小請求確認認證可用;逗號分隔的多把 Key 只要一把成功即可,逐把記錄成敗;Ollama 無 Key,改為檢查 `OLLAMA_BASE_URL` 可連線
|
||||
1. 服務名稱、模型名稱、角色資訊(個性、符合個性的英文名稱、工作內容),Comment 到 Pull Request
|
||||
1. 準備服務名稱、模型名稱、角色資訊(個性、符合個性的英文名稱、工作內容)作為本次 Pull Review 的主內容開頭;後續問題彙整完成後,會將角色介紹與所有問題一起發布在同一筆 Pull Review 底下
|
||||
2. 每個角色個別分析 Git Diff 的內容產生新問題表格(問題等級、角色名稱、問題位置或行數、修改建議)
|
||||
3. 讀取來源分支中的所有未解決舊問題(問題檔案 `.gitea/ai-review/findings.json`)加上新問題後,去除重複產生本次 PR 的問題表格(PR問題表格)覆蓋問題檔案
|
||||
4. 讀取來源分支中的排除問題檔案(`.gitea/ai-review/exclusions.json`),用來過濾 PR 問題表格中不需要處理的問題
|
||||
5. 從 PR 問題表格中取出所有舊問題,依照等級排序後 Comment 到 Pull Request
|
||||
6. 從 PR 問題表格中取出所有新問題,排除嚴重等級的問題後 Comment 到 Pull Request
|
||||
7. 從 PR 問題表格中取出所有新問題,將每個嚴重等級的問題以 Gitea 行內 review comment 標註在問題所在的檔案與行數上,留言內容為等級/審查員/建議;若問題位置無法解析出行號(例如未標行號或一次列出多個檔案),或該行不在本次 diff 範圍內導致行內留言失敗,則降級為一般 PR Comment
|
||||
5. 從 PR 問題表格中取出所有舊問題;若問題位置可解析出單一檔案行號,則放入同一筆 Pull Review 底下的 Gitea 行內 review comment,否則放入 Pull Review 主內容
|
||||
6. 從 PR 問題表格中取出所有新問題(包含非嚴重與嚴重);若問題位置可解析出單一檔案行號,則放入同一筆 Pull Review 底下的 Gitea 行內 review comment,否則放入 Pull Review 主內容
|
||||
7. 每個行內 review comment 內容包含分類、等級、審查員、建議;若批次行內留言因行號不在本次 diff 範圍等原因失敗,則將所有問題降級放入同一筆 Pull Review 主內容,避免散落成多筆 PR Comment
|
||||
8. Commit 問題檔案,只將 workspace 中實際存在的 `.gitea/ai-review/findings.json` 與 `.gitea/ai-review/exclusions.json` 覆蓋到記憶區;workspace 沒有的問題檔就略過。自動提交的 commit message 會帶上 `[ai-review-bot]`,供 workflow 判斷是否要跳過重跑
|
||||
9. 如果 PR 問題表格中有嚴重問題,則不要讓 workflow 執行成功(exit 1)
|
||||
|
||||
|
||||
+65
-1
@@ -1,6 +1,6 @@
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import { postComment, postPullReviewComment } from './gitea.js';
|
||||
import { createPullReview, postComment, postPullReviewComment } from './gitea.js';
|
||||
import { FINDINGS_PATH } from './config.js';
|
||||
import { ok, line, warn } from './log.js';
|
||||
|
||||
@@ -36,6 +36,70 @@ function inlineCommentBody(f) {
|
||||
return `**等級**:${levelText(f)}\n**審查員**:${f.role}\n**建議**:${f.suggestion}`;
|
||||
}
|
||||
|
||||
function reviewSection(title, findings) {
|
||||
if (findings.length === 0) return '';
|
||||
return `${title}\n\n${buildTable(findings)}`;
|
||||
}
|
||||
|
||||
function findingGroupLabel(f) {
|
||||
if (!f.is_new) return '舊有未解決問題';
|
||||
if (f.level === 'critical') return '新嚴重問題';
|
||||
return '新發現問題';
|
||||
}
|
||||
|
||||
export function buildReviewPayload(intro, findings, { forceBodyFindings = false } = {}) {
|
||||
const inlineComments = [];
|
||||
const bodyFindings = [];
|
||||
|
||||
for (const f of findings) {
|
||||
const loc = forceBodyFindings ? null : parseLocation(f.location);
|
||||
if (loc) {
|
||||
inlineComments.push({
|
||||
path: loc.file,
|
||||
body: `**分類**:${findingGroupLabel(f)}\n${inlineCommentBody(f)}`,
|
||||
new_position: loc.line,
|
||||
});
|
||||
} else {
|
||||
bodyFindings.push(f);
|
||||
}
|
||||
}
|
||||
|
||||
const bodyOld = bodyFindings.filter(f => !f.is_new);
|
||||
const bodyNewNonCritical = bodyFindings.filter(f => f.is_new && f.level !== 'critical');
|
||||
const bodyNewCritical = bodyFindings.filter(f => f.is_new && f.level === 'critical');
|
||||
|
||||
const sections = [
|
||||
intro,
|
||||
reviewSection(`## 📋 無法行內標註的舊有未解決問題(${bodyOld.length} 筆)`, bodyOld),
|
||||
reviewSection(`## 🔍 無法行內標註的新發現問題(${bodyNewNonCritical.length} 筆)`, bodyNewNonCritical),
|
||||
reviewSection(`## 🚨 無法行內標註的新嚴重問題(${bodyNewCritical.length} 筆)`, bodyNewCritical),
|
||||
].filter(Boolean);
|
||||
|
||||
if (inlineComments.length > 0) {
|
||||
sections.push(`## 💬 行內標註問題(${inlineComments.length} 筆)\n\n詳見本 review 底下的行內 comments。`);
|
||||
}
|
||||
|
||||
return {
|
||||
body: sections.join('\n\n'),
|
||||
comments: inlineComments,
|
||||
};
|
||||
}
|
||||
|
||||
export async function postFindingsReview(intro, findings, deps = {}) {
|
||||
const { postReview = createPullReview } = deps;
|
||||
const payload = buildReviewPayload(intro, findings);
|
||||
try {
|
||||
await postReview(payload);
|
||||
ok(`Review 發布成功 (body findings=${findings.length - payload.comments.length} inline=${payload.comments.length})`);
|
||||
} catch (e) {
|
||||
if (payload.comments.length === 0) throw e;
|
||||
warn(`Review 行內 comment 批次發布失敗,改將所有問題放入同一筆 Review body: ${e.message}`);
|
||||
const fallbackPayload = buildReviewPayload(intro, findings, { forceBodyFindings: true });
|
||||
await postReview(fallbackPayload);
|
||||
ok(`Review 發布成功 (body findings=${findings.length} inline=0)`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 寫入 findings.json。
|
||||
* 預設寫到 workspace;若提供 mirrorDir,則同步寫入另一份供 repo commit 使用。
|
||||
|
||||
+50
-1
@@ -3,7 +3,7 @@ import assert from 'node:assert/strict';
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { saveFindings, parseLocation, postNewCriticalComments } from './comments.js';
|
||||
import { saveFindings, parseLocation, buildReviewPayload, postFindingsReview, postNewCriticalComments } from './comments.js';
|
||||
import { FINDINGS_PATH } from './config.js';
|
||||
|
||||
describe('saveFindings', () => {
|
||||
@@ -185,3 +185,52 @@ describe('postNewCriticalComments', () => {
|
||||
assert.ok(issueCalls.every(b => criticalCommentPattern.test(b)));
|
||||
});
|
||||
});
|
||||
|
||||
describe('review payload', () => {
|
||||
const intro = '## Reviewers\n\nLeo / Maya';
|
||||
const findings = [
|
||||
{ level: 'warning', role: 'Leo', location: 'app/a.js:1', suggestion: '改善命名', is_new: true },
|
||||
{ level: 'critical', role: 'Rex', location: 'app/b.js:5', suggestion: '修正權限檢查', is_new: true },
|
||||
{ level: 'critical', role: 'Maya', location: 'app/c.js', suggestion: '補上交易保護', is_new: true },
|
||||
{ level: 'info', role: 'Bard', location: 'app/d.js:9', suggestion: '補註解', is_new: false },
|
||||
];
|
||||
|
||||
it('builds one review body with role intro and inline comments for every located finding', () => {
|
||||
const payload = buildReviewPayload(intro, findings);
|
||||
assert.match(payload.body, /Reviewers/);
|
||||
assert.doesNotMatch(payload.body, /改善命名/);
|
||||
assert.match(payload.body, /無法行內標註的新嚴重問題/);
|
||||
assert.equal(payload.comments.length, 3);
|
||||
assert.deepEqual(payload.comments.map(c => c.path), ['app/a.js', 'app/b.js', 'app/d.js']);
|
||||
assert.deepEqual(payload.comments.map(c => c.new_position), [1, 5, 9]);
|
||||
assert.match(payload.comments[0].body, /新發現問題/);
|
||||
assert.match(payload.comments[1].body, /新嚴重問題/);
|
||||
assert.match(payload.comments[2].body, /舊有未解決問題/);
|
||||
});
|
||||
|
||||
it('posts findings as a single pull review', async () => {
|
||||
const calls = [];
|
||||
await postFindingsReview(intro, findings, {
|
||||
postReview: async (payload) => { calls.push(payload); },
|
||||
});
|
||||
assert.equal(calls.length, 1);
|
||||
assert.match(calls[0].body, /Reviewers/);
|
||||
assert.equal(calls[0].comments.length, 3);
|
||||
});
|
||||
|
||||
it('falls back to one review body when inline comments are rejected', async () => {
|
||||
const calls = [];
|
||||
await postFindingsReview(intro, findings, {
|
||||
postReview: async (payload) => {
|
||||
calls.push(payload);
|
||||
if (calls.length === 1) throw new Error('line not in diff');
|
||||
},
|
||||
});
|
||||
assert.equal(calls.length, 2);
|
||||
assert.equal(calls[1].comments.length, 0);
|
||||
assert.match(calls[1].body, /改善命名/);
|
||||
assert.match(calls[1].body, /修正權限檢查/);
|
||||
assert.match(calls[1].body, /補上交易保護/);
|
||||
assert.match(calls[1].body, /補註解/);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -114,6 +114,13 @@ describe('getLLMConfig', () => {
|
||||
assert.equal(shouldSkipOpenCodeTLSVerify(), false);
|
||||
});
|
||||
|
||||
it('skips OpenCode TLS verification for any value other than false', () => {
|
||||
for (const value of ['', '0', 'true', 'yes']) {
|
||||
process.env.OPENCODE_SKIP_TLS_VERIFY = value;
|
||||
assert.equal(shouldSkipOpenCodeTLSVerify(), true);
|
||||
}
|
||||
});
|
||||
|
||||
it('openai takes priority over gemini when both set', () => {
|
||||
process.env.OPENAI_API_KEY = 'sk-test';
|
||||
process.env.GEMINI_API_KEY = 'gemini-key';
|
||||
|
||||
+17
-7
@@ -119,20 +119,30 @@ export async function postComment(body) {
|
||||
}
|
||||
|
||||
/**
|
||||
* 在 PR 指定檔案的指定行數發布行內 review comment(標註程式碼位置)。
|
||||
* 透過 Gitea 的 pull reviews API,以 new_position 對應新版檔案的行號。
|
||||
* 若該行不在 diff 範圍內,Gitea 會回傳錯誤,由呼叫端決定是否降級為一般 comment。
|
||||
* 建立一筆 Pull Review。body 會成為 review 主內容,comments 會成為同一筆 review 底下的行內 comments。
|
||||
*/
|
||||
export async function postPullReviewComment({ path: filePath, line, body }) {
|
||||
export async function createPullReview({ body, comments = [], event = 'COMMENT' }) {
|
||||
const resp = await axios.post(
|
||||
api(`/repos/${GITEA_REPOSITORY}/pulls/${PR_NUMBER}/reviews`),
|
||||
{
|
||||
commit_id: PR_HEAD_SHA || undefined,
|
||||
event: 'COMMENT',
|
||||
body: '',
|
||||
comments: [{ path: filePath, body, new_position: line }],
|
||||
event,
|
||||
body,
|
||||
comments,
|
||||
},
|
||||
{ headers: headers(GITEA_COMMENT_TOKEN || GITEA_TOKEN), timeout: 30000, httpsAgent },
|
||||
);
|
||||
return resp.data;
|
||||
}
|
||||
|
||||
/**
|
||||
* 在 PR 指定檔案的指定行數發布行內 review comment(標註程式碼位置)。
|
||||
* 透過 Gitea 的 pull reviews API,以 new_position 對應新版檔案的行號。
|
||||
* 若該行不在 diff 範圍內,Gitea 會回傳錯誤,由呼叫端決定是否降級為一般 comment。
|
||||
*/
|
||||
export async function postPullReviewComment({ path: filePath, line, body }) {
|
||||
return createPullReview({
|
||||
body: '',
|
||||
comments: [{ path: filePath, body, new_position: line }],
|
||||
});
|
||||
}
|
||||
|
||||
+19
-1
@@ -1,7 +1,7 @@
|
||||
import { describe, it, afterEach, mock } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import axios from 'axios';
|
||||
import { getPRDiff, filterDiff, postComment, postPullReviewComment, getCommitMessageBySha, getBranchHeadCommitMessage, shouldSkipBotCommit, getBotReviewOutcome } from './gitea.js';
|
||||
import { getPRDiff, filterDiff, postComment, createPullReview, postPullReviewComment, getCommitMessageBySha, getBranchHeadCommitMessage, shouldSkipBotCommit, getBotReviewOutcome } from './gitea.js';
|
||||
|
||||
afterEach(() => mock.restoreAll());
|
||||
|
||||
@@ -77,6 +77,24 @@ describe('gitea', () => {
|
||||
assert.ok(capturedOpts.headers['Authorization'].startsWith('token '));
|
||||
});
|
||||
|
||||
it('createPullReview posts body and comments in one pull review', async () => {
|
||||
let capturedUrl, capturedBody;
|
||||
mock.method(axios, 'post', async (url, body) => {
|
||||
capturedUrl = url;
|
||||
capturedBody = body;
|
||||
return { data: { id: 8 } };
|
||||
});
|
||||
const result = await createPullReview({
|
||||
body: 'review body',
|
||||
comments: [{ path: 'app/a.js', body: 'inline', new_position: 3 }],
|
||||
});
|
||||
assert.deepEqual(result, { id: 8 });
|
||||
assert.ok(capturedUrl.endsWith('/reviews'));
|
||||
assert.equal(capturedBody.event, 'COMMENT');
|
||||
assert.equal(capturedBody.body, 'review body');
|
||||
assert.deepEqual(capturedBody.comments, [{ path: 'app/a.js', body: 'inline', new_position: 3 }]);
|
||||
});
|
||||
|
||||
it('postPullReviewComment propagates axios errors', async () => {
|
||||
mock.method(axios, 'post', async () => { throw new Error('not in diff'); });
|
||||
await assert.rejects(() => postPullReviewComment({ path: 'a.js', line: 1, body: 'x' }), /not in diff/);
|
||||
|
||||
+6
-13
@@ -1,9 +1,9 @@
|
||||
import path from 'path';
|
||||
import { GITEA_REPOSITORY, PR_NUMBER, PR_HEAD_BRANCH, PR_BASE_BRANCH, getLLMConfig, FINDINGS_PATH, EXCLUSIONS_PATH } from './config.js';
|
||||
import { loadRoles, getRoleIntro } from './roles.js';
|
||||
import { getPRDiff, postComment, getCommitMessageBySha, getBotReviewOutcome, shouldSkipBotCommit } from './gitea.js';
|
||||
import { getPRDiff, getCommitMessageBySha, getBotReviewOutcome, shouldSkipBotCommit } from './gitea.js';
|
||||
import { analyzeWithRole, loadOldFindings, mergeFindings, sortByLevel, deduplicateWithAI, loadExclusions, applyExclusions, filterFalsePositivesWithAI } from './findings.js';
|
||||
import { saveFindings, postOldFindingsComment, postNewNonCriticalComment, postNewCriticalComments } from './comments.js';
|
||||
import { saveFindings, postFindingsReview } from './comments.js';
|
||||
import { cloneRepo, commitAndPush, getRepoState } from './git.js';
|
||||
import { validateJSONArrayFile, ensureJSONArrayFileExists } from './json.js';
|
||||
import { runPreflight } from './preflight.js';
|
||||
@@ -63,13 +63,8 @@ async function main() {
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
try {
|
||||
const intro = getRoleIntro(roles) + `\n\n> 🔍 服務:${provider} 模型:${model}`;
|
||||
await postComment(intro);
|
||||
ok('角色介紹 comment 發布成功');
|
||||
} catch (e) {
|
||||
warn(`comment 發布失敗(繼續執行): ${e.message}`);
|
||||
}
|
||||
const reviewIntro = getRoleIntro(roles) + `\n\n> 🔍 服務:${provider} 模型:${model}`;
|
||||
ok('角色介紹 Review 內容已準備');
|
||||
|
||||
step('Step2', 'Findings 產生');
|
||||
const results = await Promise.allSettled(roles.map(role => analyzeWithRole(role, diff)));
|
||||
@@ -111,12 +106,10 @@ async function main() {
|
||||
const reviewDir = repoDir || WORKSPACE;
|
||||
saveFindings(WORKSPACE, filtered, reviewDir);
|
||||
try {
|
||||
await postOldFindingsComment(filtered);
|
||||
await postNewNonCriticalComment(filtered);
|
||||
await postNewCriticalComments(filtered);
|
||||
await postFindingsReview(reviewIntro, filtered);
|
||||
ok('Step5 完成');
|
||||
} catch (e) {
|
||||
warn(`comment 發布失敗(繼續執行): ${e.message}`);
|
||||
warn(`Review 發布失敗(繼續執行): ${e.message}`);
|
||||
}
|
||||
|
||||
step('Step6', 'JSON 格式驗證');
|
||||
|
||||
@@ -199,6 +199,23 @@ describe('verifyLLM', () => {
|
||||
assert.equal(agents[1].options.rejectUnauthorized, false);
|
||||
});
|
||||
|
||||
|
|
||||
it('passes an insecure https agent for opencode when TLS skip is explicitly true', async () => {
|
||||
clearLLMEnv();
|
||||
process.env.OPENCODE_BASE_URL = 'https://opencode.local:4096';
|
||||
process.env.OPENCODE_SKIP_TLS_VERIFY = 'true';
|
||||
const agents = [];
|
||||
mock.method(axios, 'get', async (url, opts) => {
|
||||
agents.push(opts.httpsAgent);
|
||||
if (url.endsWith('/global/health')) return { data: { healthy: true } };
|
||||
return { data: { providers: [{ id: 'google', models: { 'gemini-2.5-flash': { id: 'gemini-2.5-flash' } } }] } };
|
||||
});
|
||||
const result = await verifyLLM();
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(agents.length, 2);
|
||||
assert.equal(agents[0].options.rejectUnauthorized, false);
|
||||
assert.equal(agents[1].options.rejectUnauthorized, false);
|
||||
});
|
||||
|
||||
it('does not pass an insecure https agent for opencode when TLS verification is enabled', async () => {
|
||||
clearLLMEnv();
|
||||
process.env.OPENCODE_BASE_URL = 'https://opencode.local:4096';
|
||||
|
||||
Reference in New Issue
Block a user
分類:新嚴重問題
等級:🔴 嚴重
審查員:Assassin
建議:此測試進一步確認了
OPENCODE_SKIP_TLS_VERIFY = 'true'會導致 HTTPS 代理設定為rejectUnauthorized: false,即跳過 TLS 憑證驗證。這是一個嚴重的「不安全預設」或「不安全配置」問題。雖然這是一個明確的設定,但跳過 TLS 驗證會使應用程式容易受到中間人(MITM)攻擊。攻擊者可以在應用程式與 OpenCode 服務之間偽造伺服器身份,攔截、竊聽或篡改所有通訊內容,進而竊取機密資訊或注入惡意指令。
建議除非在極端受控的環境下,否則應避免跳過 TLS 驗證。如果確實需要此功能,應確保其使用受到嚴格的審查和限制,並在文件上明確標示其安全風險。更安全的做法是配置正確的憑證信任鏈,而不是禁用驗證。