Compare commits

...

4 Commits

5 changed files with 116 additions and 45 deletions
+20
View File
@@ -183,5 +183,25 @@
"role": "Maya", "role": "Maya",
"location": "app/roles.js", "location": "app/roles.js",
"suggestion": "roles.js 依賴容器內固定路徑 /action/app/prompts/roles,單元測試環境無法存取,且邏輯為簡單 YAML 讀取與字串拼接" "suggestion": "roles.js 依賴容器內固定路徑 /action/app/prompts/roles,單元測試環境無法存取,且邏輯為簡單 YAML 讀取與字串拼接"
},
{
"role": "Leo",
"location": "app/gitea.js",
"suggestion": "gitea.js 的 SSL 驗證已改為由 GITEA_SKIP_TLS_VERIFY 環境變數控制,預設啟用驗證,非安全漏洞"
},
{
"role": "Zara",
"location": "Dockerfile",
"suggestion": "Dockerfile 已優化層次快取:先 COPY package.json 再 npm install,最後才 COPY 其餘檔案"
},
{
"role": "Aria",
"location": "app/package.json",
"suggestion": "test 腳本已改為 node --test *.test.js,在 app/ 目錄下執行可自動發現所有測試檔案"
},
{
"role": "Zara",
"location": "app/main.js",
"suggestion": "deduplicateWithAI 和 filterFalsePositivesWithAI 為循序依賴流程(去重後才能過濾),無法平行化"
} }
] ]
+24 -38
View File
@@ -1,30 +1,30 @@
[ [
{
"level": "critical",
"role": "Maya",
"location": "app/gitea.js",
"suggestion": "`app/gitea.js` 模組負責與 Gitea API 進行所有互動,包括獲取 PR Diff 和發布評論。這些網路操作是 Action 運作的基礎,但目前缺乏單元測試。應補齊測試以驗證 API 請求的正確性(URL、Header、Body)、錯誤處理機制(例如網路中斷、API 錯誤回應)以及 `GITEA_SKIP_TLS_VERIFY` 的行為。",
"is_new": true
},
{ {
"level": "critical", "level": "critical",
"role": "Leo", "role": "Leo",
"location": "app/gitea.js:10", "location": "app/gitea.js:10",
"suggestion": "在 `app/gitea.js` 中,`https.Agent` 設定了 `rejectUnauthorized: false`,這會禁用 SSL/TLS 憑證驗證。這是一個嚴重的安全漏洞,會使應用程式容易受到中間人攻擊,並嚴重影響系統的安全性與可維護性。強烈建議移除此設定,或在必要時配置正確的憑證信任鏈。", "suggestion": "在 `app/gitea.js` 中,`https.Agent` 設定了 `rejectUnauthorized: false`,這會禁用 SSL/TLS 憑證驗證。這是一個嚴重的安全漏洞,會使應用程式容易受到中間人攻擊,並嚴重影響系統的安全性與可維護性。強烈建議移除此設定,或在必要時配置正確的憑證信任鏈。",
"is_new": true "is_new": false
},
{
"level": "critical",
"role": "Aria",
"location": "Dockerfile",
"suggestion": "檔案結尾缺少換行符號(newline character),這不符合 POSIX 慣例,建議在檔案末尾新增一個空行。",
"is_new": true
},
{
"level": "critical",
"role": "Aria",
"location": "entrypoint.sh",
"suggestion": "檔案結尾缺少換行符號(newline character),這不符合 POSIX 慣例,建議在檔案末尾新增一個空行。",
"is_new": true
}, },
{ {
"level": "warning", "level": "warning",
"role": "Leo", "role": "Leo",
"location": "Dockerfile:2", "location": "app/roles.js:4",
"suggestion": "在 `Dockerfile` 中,基礎映像檔使用了 `FROM alpine` 而沒有指定具體的版本標籤(例如 `alpine:3.18`)。這可能導致未來 `alpine:latest` 更新時,建置環境發生非預期的變更,影響建置的可重現性和穩定性。為了長期維護性,建議明確指定基礎映像檔的版本。", "suggestion": "在 `ROLES_DIR` 常數中,使用了硬編碼的路徑 `/action/app/prompts/roles`。這使得在 Docker 容器外部進行本地開發或測試時,需要額外配置才能正確載入角色定義。建議將此路徑設定為可配置的環境變數,或使用相對路徑並在 `main.js` 中傳入基礎路徑,以提高模組的彈性和可測試性。",
"is_new": true
},
{
"level": "warning",
"role": "Zara",
"location": "app/main.js:60",
"suggestion": "`analyzeWithRole` 函式在迴圈中為每個角色依序呼叫 LLM 進行分析。由於 LLM 呼叫是高延遲操作,這種序列化執行會顯著增加整個 Code Review Action 的總執行時間。考慮使用 `Promise.all` 等方式平行化這些 LLM 呼叫,以縮短總等待時間。",
"is_new": true "is_new": true
}, },
{ {
@@ -32,34 +32,20 @@
"role": "Zara", "role": "Zara",
"location": "Dockerfile", "location": "Dockerfile",
"suggestion": "建議調整 Dockerfile 的層次,將 `COPY app/package.json app/package-lock.json /action/app/` 放在 `npm install` 之前,然後再 `COPY app/ /action/app/`。這樣可以利用 Docker 的層次快取,當 `package.json` 或 `package-lock.json` 未變更時,`npm install` 步驟就不會重複執行,顯著加快 Docker 映像檔的建置速度。", "suggestion": "建議調整 Dockerfile 的層次,將 `COPY app/package.json app/package-lock.json /action/app/` 放在 `npm install` 之前,然後再 `COPY app/ /action/app/`。這樣可以利用 Docker 的層次快取,當 `package.json` 或 `package-lock.json` 未變更時,`npm install` 步驟就不會重複執行,顯著加快 Docker 映像檔的建置速度。",
"is_new": true "is_new": false
}, },
{ {
"level": "warning", "level": "warning",
"role": "Maya", "role": "Aria",
"location": "app/main.js", "location": "app/package.json:5",
"suggestion": "儘管各單元模組(`config`, `git`, `llm`)已有良好的單元測試,但 `app/main.js` 作為整個 Action 的協調者,其整合邏輯和端到端流程缺乏測試。特別是 `TODO.md` 中提到的「阻擋嚴重問題 PR(exit 1)」等關鍵行為,應透過整合測試或端到端測試來驗證,確保各模組協同運作正確,且整體流程符合預期。", "suggestion": "`package.json` 中的 `test` 腳本 (`node --test *.test.js`) 僅會執行 `app/` 目錄下的測試檔案。建議修改為 `node --test app/**/*.test.js` 或使用更完善的測試框架(如 `mocha` 或 `jest`),以確保所有測試檔案都能被自動發現並執行。",
"is_new": true
},
{
"level": "warning",
"role": "Maya",
"location": "app/comments.js",
"suggestion": "`app/comments.js` 模組負責生成評論內容和儲存 findings 檔案。其中 `buildTable` 等函式負責 Markdown 表格的生成,以及 `postOldFindingsComment`, `postNewNonCriticalComment`, `postNewCriticalComments` 中的過濾邏輯,應補齊單元測試,以確保評論內容的格式正確性及問題分類的準確性。",
"is_new": true
},
{
"level": "warning",
"role": "Maya",
"location": "app/roles.js",
"suggestion": "`app/roles.js` 模組負責載入和處理角色定義。`loadRoles` 函式應增加單元測試,以驗證其能正確解析 YAML 檔案,並處理檔案不存在或格式錯誤等異常情況。`getRoleIntro` 函式也應測試其生成的 Markdown 介紹內容是否符合預期格式。",
"is_new": true "is_new": true
}, },
{ {
"level": "info", "level": "info",
"role": "Maya", "role": "Zara",
"location": "app/package.json", "location": "app/main.js:72, app/main.js:78",
"suggestion": "目前的 `test` 腳本 (`node --test git.test.js config.test.js llm.test.js`) 僅涵蓋了部分單元測試。建議更新此腳本,使其能自動發現並執行所有 `*.test.js` 檔案,以確保所有新增的測試都能被納入 CI 流程中執行。", "suggestion": "`deduplicateWithAI` 和 `filterFalsePositivesWithAI` 函式也涉及 LLM 呼叫,它們在主流程中依序執行。雖然這些是單次呼叫,但其延遲仍會累積。在設計上,可以考慮是否有可能將這些步驟與其他非依賴的任務平行化,或或評估其對整體效能的影響。",
"is_new": true "is_new": true
} }
] ]
+64
View File
@@ -0,0 +1,64 @@
import { describe, it, afterEach, mock } from 'node:test';
import assert from 'node:assert/strict';
import axios from 'axios';
// gitea.js reads env vars at module load time (ESM cache), so we test
// the actual values baked in at import time and verify behavior via axios mocks.
afterEach(() => mock.restoreAll());
describe('gitea', async () => {
const { getPRDiff, postComment } = await import('./gitea.js');
it('getPRDiff calls Gitea diff API with Authorization header', async () => {
let capturedUrl, capturedOpts;
mock.method(axios, 'get', async (url, opts) => {
capturedUrl = url;
capturedOpts = opts;
return { data: 'diff content' };
});
const result = await getPRDiff();
assert.equal(result, 'diff content');
assert.ok(capturedUrl.includes('/api/v1/repos/'));
assert.ok(capturedUrl.endsWith('.diff'));
assert.ok(capturedOpts.headers['Authorization'].startsWith('token '));
assert.equal(capturedOpts.headers['Content-Type'], 'application/json');
});
it('postComment calls Gitea issues comments API with body', async () => {
let capturedUrl, capturedBody, capturedOpts;
mock.method(axios, 'post', async (url, body, opts) => {
capturedUrl = url;
capturedBody = body;
capturedOpts = opts;
return { data: { id: 1 } };
});
const result = await postComment('hello world');
assert.deepEqual(result, { id: 1 });
assert.ok(capturedUrl.includes('/api/v1/repos/'));
assert.ok(capturedUrl.endsWith('/comments'));
assert.equal(capturedBody.body, 'hello world');
assert.ok(capturedOpts.headers['Authorization'].startsWith('token '));
});
it('does not set httpsAgent by default (GITEA_SKIP_TLS_VERIFY not true)', async () => {
let capturedOpts;
mock.method(axios, 'get', async (_url, opts) => {
capturedOpts = opts;
return { data: '' };
});
await getPRDiff();
// httpsAgent is undefined when GITEA_SKIP_TLS_VERIFY !== 'true'
assert.equal(capturedOpts.httpsAgent, undefined);
});
it('getPRDiff propagates axios errors', async () => {
mock.method(axios, 'get', async () => { throw new Error('network error'); });
await assert.rejects(() => getPRDiff(), /network error/);
});
it('postComment propagates axios errors', async () => {
mock.method(axios, 'post', async () => { throw new Error('api error'); });
await assert.rejects(() => postComment('test'), /api error/);
});
});
+6 -6
View File
@@ -52,13 +52,13 @@ async function main() {
// Step2: 各角色分析 diff 產生新 findings // Step2: 各角色分析 diff 產生新 findings
console.log('\n📊 Step2: Findings 產生'); console.log('\n📊 Step2: Findings 產生');
const results = await Promise.allSettled(roles.map(role => analyzeWithRole(role, diff)));
const newFindings = []; const newFindings = [];
for (const role of roles) { for (let i = 0; i < results.length; i++) {
try { if (results[i].status === 'fulfilled') {
const found = await analyzeWithRole(role, diff); newFindings.push(...results[i].value);
newFindings.push(...found); } else {
} catch (e) { console.log(` ⚠️ [${roles[i].name}] 分析失敗(跳過): ${results[i].reason?.message}`);
console.log(` ⚠️ [${role.name}] 分析失敗(跳過): ${e.message}`);
} }
} }
console.log(` Step2 完成: 新 findings 總計 ${newFindings.length}`); console.log(` Step2 完成: 新 findings 總計 ${newFindings.length}`);
+2 -1
View File
@@ -1,8 +1,9 @@
import fs from 'fs'; import fs from 'fs';
import path from 'path'; import path from 'path';
import { fileURLToPath } from 'url';
import yaml from 'js-yaml'; import yaml from 'js-yaml';
const ROLES_DIR = '/action/app/prompts/roles'; const ROLES_DIR = path.join(fileURLToPath(import.meta.url), '..', 'prompts', 'roles');
export function loadRoles() { export function loadRoles() {
return fs.readdirSync(ROLES_DIR) return fs.readdirSync(ROLES_DIR)