old-actions/code-review
2
0
Fork 0
Code Pull Requests Actions Releases 2 Activity

調整 AI review 以單一 Review 發布統計與 comments #28

Merged
jiantw83 merged 17 commits from ai-review-resolve/20260622100149 into develop 2026-06-22 10:31:46 +00:00
Conversation 12 Commits 17 Files Changed 17
+24
2 changed files with 24 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit a60fb8d168 - Show all commits
app/config.test.js
+7
View File
@@ -114,6 +114,13 @@ describe('getLLMConfig', () => {
assert.equal(shouldSkipOpenCodeTLSVerify(), false); assert.equal(shouldSkipOpenCodeTLSVerify(), false);
admin commented 2026-06-22 10:09:21 +00:00
Review
Copy Link
Copy Source

嚴重等級：🔴 嚴重
審查員：Mage
問題：app/config.test.js:114
建議：新加入的測試案例 it('skips OpenCode TLS verification for empty string and non-false values', ...) 預期 shouldSkipOpenCodeTLSVerify() 函式在 OPENCODE_SKIP_TLS_VERIFY 環境變數為空字串 '' 或 '0' 時，會回傳 true。

然而，根據常見的環境變數布林值解析邏輯，以及 app/preflight.test.js 中現有的相關測試（例如未設定時為 false，設定為 'false' 時為 false），shouldSkipOpenCodeTLSVerify() 函式（此 PR 未修改其內容）很可能不會將 '' 或 '0' 視為 true。

這造成了測試預期與函式實際行為之間的邏輯不一致。請確認以下其中一項：

  1. 如果 shouldSkipOpenCodeTLSVerify() 函式確實應該將 '' 和 '0' 視為 true，則該函式本身需要被修改以符合此行為。
  2. 如果 shouldSkipOpenCodeTLSVerify() 函式不應將 '' 和 '0' 視為 true，則此測試案例的斷言應被修正，或移除對 '' 和 '0' 的測試。
**嚴重等級**：🔴 嚴重 **審查員**：Mage **問題**：app/config.test.js:114 **建議**：新加入的測試案例 `it('skips OpenCode TLS verification for empty string and non-false values', ...)` 預期 `shouldSkipOpenCodeTLSVerify()` 函式在 `OPENCODE_SKIP_TLS_VERIFY` 環境變數為空字串 `''` 或 `'0'` 時，會回傳 `true`。 然而，根據常見的環境變數布林值解析邏輯，以及 `app/preflight.test.js` 中現有的相關測試（例如未設定時為 `false`，設定為 `'false'` 時為 `false`），`shouldSkipOpenCodeTLSVerify()` 函式（此 PR 未修改其內容）很可能不會將 `''` 或 `'0'` 視為 `true`。 這造成了測試預期與函式實際行為之間的邏輯不一致。請確認以下其中一項： 1. 如果 `shouldSkipOpenCodeTLSVerify()` 函式確實應該將 `''` 和 `'0'` 視為 `true`，則該函式本身需要被修改以符合此行為。 2. 如果 `shouldSkipOpenCodeTLSVerify()` 函式不應將 `''` 和 `'0'` 視為 `true`，則此測試案例的斷言應被修正，或移除對 `''` 和 `'0'` 的測試。
}); });
it('skips OpenCode TLS verification for empty string and non-false values', () => {
admin commented 2026-06-22 10:04:21 +00:00
Review
Copy Link
Copy Source

嚴重等級：🟡 警告
審查員：Mage
問題：app/config.test.js:117
建議：環境變數 OPENCODE_SKIP_TLS_VERIFY 被設定為空字串 ('') 時，系統會將其解讀為 true，導致跳過 TLS 驗證。這與一般布林環境變數的慣例（空字串通常視為 false 或未設定）不符，可能造成使用者誤解，意外地啟用不安全的設定。建議調整 shouldSkipOpenCodeTLSVerify 的邏輯，將空字串視為 false，或明確要求使用者輸入 true 或 1 來啟用跳過驗證。

**嚴重等級**：🟡 警告 **審查員**：Mage **問題**：app/config.test.js:117 **建議**：環境變數 `OPENCODE_SKIP_TLS_VERIFY` 被設定為空字串 (`''`) 時，系統會將其解讀為 `true`，導致跳過 TLS 驗證。這與一般布林環境變數的慣例（空字串通常視為 `false` 或未設定）不符，可能造成使用者誤解，意外地啟用不安全的設定。建議調整 `shouldSkipOpenCodeTLSVerify` 的邏輯，將空字串視為 `false`，或明確要求使用者輸入 `true` 或 `1` 來啟用跳過驗證。
for (const value of ['', '0', 'true', 'yes']) {
process.env.OPENCODE_SKIP_TLS_VERIFY = value;
admin commented 2026-06-22 10:16:15 +00:00
Review
Copy Link
Copy Source

嚴重等級：🟡 警告
審查員：Maya
問題：app/config.test.js:119
建議：函式 shouldSkipOpenCodeTLSVerify 的邏輯是只要環境變數 OPENCODE_SKIP_TLS_VERIFY 的值不是 'false' 就回傳 true。目前的測試案例涵蓋了 '', '0', 'true', 'yes'。為了更全面地驗證此寬鬆的判斷邏輯，請考慮新增測試案例，例如 '1'、'on' 或其他任意非 'false' 的字串，以確保其行為符合預期。

**嚴重等級**：🟡 警告 **審查員**：Maya **問題**：app/config.test.js:119 **建議**：函式 `shouldSkipOpenCodeTLSVerify` 的邏輯是只要環境變數 `OPENCODE_SKIP_TLS_VERIFY` 的值不是 `'false'` 就回傳 `true`。目前的測試案例涵蓋了 `''`, `'0'`, `'true'`, `'yes'`。為了更全面地驗證此寬鬆的判斷邏輯，請考慮新增測試案例，例如 `'1'`、`'on'` 或其他任意非 `'false'` 的字串，以確保其行為符合預期。
assert.equal(shouldSkipOpenCodeTLSVerify(), true);
}
});
it('openai takes priority over gemini when both set', () => { it('openai takes priority over gemini when both set', () => {
process.env.OPENAI_API_KEY = 'sk-test'; process.env.OPENAI_API_KEY = 'sk-test';
process.env.GEMINI_API_KEY = 'gemini-key'; process.env.GEMINI_API_KEY = 'gemini-key';
app/preflight.test.js
+17
View File
@@ -199,6 +199,23 @@ describe('verifyLLM', () => {
assert.equal(agents[1].options.rejectUnauthorized, false); assert.equal(agents[1].options.rejectUnauthorized, false);
admin commented 2026-06-22 10:16:15 +00:00
Review
Copy Link
Copy Source

嚴重等級：🟡 警告
審查員：Maya
問題：app/preflight.test.js:199
建議：函式 verifyLLM 在 OPENCODE_SKIP_TLS_VERIFY 為 true 時會傳遞不安全的 HTTPS Agent。config.test.js 中的 shouldSkipOpenCodeTLSVerify 測試顯示，''、'0'、'yes' 也會導致跳過 TLS 驗證。請在 preflight.test.js 中新增測試案例，驗證當 OPENCODE_SKIP_TLS_VERIFY 設定為這些值時，httpsAgent 是否也能正確地設定 rejectUnauthorized: false。

**嚴重等級**：🟡 警告 **審查員**：Maya **問題**：app/preflight.test.js:199 **建議**：函式 `verifyLLM` 在 `OPENCODE_SKIP_TLS_VERIFY` 為 `true` 時會傳遞不安全的 HTTPS Agent。`config.test.js` 中的 `shouldSkipOpenCodeTLSVerify` 測試顯示，`''`、`'0'`、`'yes'` 也會導致跳過 TLS 驗證。請在 `preflight.test.js` 中新增測試案例，驗證當 `OPENCODE_SKIP_TLS_VERIFY` 設定為這些值時，`httpsAgent` 是否也能正確地設定 `rejectUnauthorized: false`。
}); });
admin commented 2026-06-22 10:23:25 +00:00
Review
Copy Link
Copy Source

嚴重等級：🔴 嚴重
審查員：Assassin
問題：app/preflight.test.js:201
建議：此測試明確證實了 OPENCODE_SKIP_TLS_VERIFY 環境變數的寬鬆判斷邏輯，導致 OpenCode LLM 連線的 TLS 驗證容易被關閉。這是「關閉 TLS 驗證」的不安全預設，極大地增加了中間人攻擊的風險。攻擊者可以利用此漏洞，在 LLM 服務通訊中插入惡意代理，竊取敏感資料或篡改 LLM 的行為。請立即修正 app/config.js 中 shouldSkipOpenCodeTLSVerify 的邏輯，使其僅在明確意圖下才關閉 TLS 驗證。

**嚴重等級**：🔴 嚴重 **審查員**：Assassin **問題**：app/preflight.test.js:201 **建議**：此測試明確證實了 `OPENCODE_SKIP_TLS_VERIFY` 環境變數的寬鬆判斷邏輯，導致 OpenCode LLM 連線的 TLS 驗證容易被關閉。這是「關閉 TLS 驗證」的不安全預設，極大地增加了中間人攻擊的風險。攻擊者可以利用此漏洞，在 LLM 服務通訊中插入惡意代理，竊取敏感資料或篡改 LLM 的行為。請立即修正 `app/config.js` 中 `shouldSkipOpenCodeTLSVerify` 的邏輯，使其僅在明確意圖下才關閉 TLS 驗證。
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 () => { it('does not pass an insecure https agent for opencode when TLS verification is enabled', async () => {
clearLLMEnv(); clearLLMEnv();
process.env.OPENCODE_BASE_URL = 'https://opencode.local:4096'; process.env.OPENCODE_BASE_URL = 'https://opencode.local:4096';