Merge pull request 'feat: 前置驗證納入 git push 認證檢查' (#10) from feat/preflight-auth-check into develop

Reviewed-on: #10

.gitea/ai-review/exclusions.json

@@ -351,5 +351,35 @@
         "location":  "Dockerfile, app/git.js, app/gitea.js",
         "suggestion":  "此變更引入了新的代理（agent）相關路徑（例如 `.agents/` 和 `AGENTS.md`），並在 `Dockerfile` 的 `COPY` 指令、`app/git.js` 中的 `SYNC_PATHS`、`FORCE_SYNC_FILE_PATHS`、`SYNC_TREE_PATHS` 陣列，以及 `app/gitea.js` 的 `filterDiff` 陣列中重複添加了這些路徑。這種模式導致了程式碼重複，每次新增一個代理都需要手動修改多個檔案和多個列表，增加了維護成本和出錯的可能性。建議考慮引入一個集中的設定檔或機制，例如透過掃描特定目錄來動態生成這些路徑列表，以提高模組化和可擴展性。",
         "is_new":  true
+    },
+    {
+        "role":  "Rex",
+        "location":  "app/preflight.js:12",
+        "suggestion":  "程式碼中根據 `GITEA_SKIP_TLS_VERIFY` 環境變數來禁用 TLS 憑證驗證 (`rejectUnauthorized: false`)，這會使應用程式容易受到中間人 (Man-in-the-Middle, MITM) 攻擊。攻擊者可能在不被察覺的情況下攔截和修改與 Gitea 伺服器的通訊。建議移除此功能，或確保在任何生產環境中永不啟用。如果 Gitea 伺服器使用自簽憑證，應將其憑證加入信任儲存區，而非禁用驗證。"
+    },
+    {
+        "role":  "Leo",
+        "location":  "app/preflight.js:56",
+        "suggestion":  "函式 `verifyLLM` 處理了多種 LLM 供應商的驗證邏輯（Ollama、Claude、OpenAI 相容等），導致其長度較長且複雜度較高。建議將不同供應商的驗證邏輯拆分成獨立的輔助函式（例如 `_verifyOllama`、`_verifyOpenAICompatible`），以提高模組化程度和可讀性。"
+    },
+    {
+        "role":  "Zara",
+        "location":  "app/preflight.js:70-82",
+        "suggestion":  "在 `verifyLLM` 函式中，當配置了多個 LLM API Key 時，系統會依序嘗試驗證每個 Key，每個嘗試都有 30 秒的逾時時間。如果前幾個 Key 驗證失敗，這可能導致顯著的累積延遲。雖然這是為了找到一個可用的 Key，但若 Key 數量多且網路不穩定，可能會造成啟動時間過長。可以考慮縮短單次 Key 驗證的逾時時間，或在特定情況下提供更快的失敗機制。"
+    },
+    {
+        "role":  "Rex",
+        "location":  "app/preflight.js:100",
+        "suggestion":  "在記錄 LLM API 驗證失敗時，直接輸出了錯誤訊息 `e.message`。雖然通常情況下 `e.message` 不會包含敏感資訊，但為了最佳安全實踐，建議審查 LLM 服務提供商的錯誤訊息格式，確保其中不會意外洩漏 API 金鑰或其他敏感請求內容。若有疑慮，應對錯誤訊息進行消毒或僅記錄高層次的錯誤類型。"
+    },
+    {
+        "role":  "Aria",
+        "location":  "app/preflight.js:30",
+        "suggestion":  "在 `checkRequiredEnv`、`verifyGiteaToken` 和 `verifyCommentToken` 等函式中，預設參數直接引用了從 `config.js` 匯入的常數。雖然這在功能上可行，但為了提高程式碼的清晰度和一致性，建議考慮以下兩種方式之一：1. 將所有配置值作為明確的參數從呼叫端傳入。2. 讓函式直接從 `config.js` 模組中讀取這些值，而不是透過預設參數。"
+    },
+    {
+        "role":  "Maya",
+        "location":  "app/preflight.js:107",
+        "suggestion":  "在 `verifyLLM` 函數中，呼叫 `axios.post` 時缺少 `httpsAgent` 選項。這會導致即使設定了 `GITEA_SKIP_TLS_VERIFY`，LLM 的 API 請求仍可能因 TLS 憑證問題而失敗。請將 `httpsAgent` 傳遞給 `axios.post` 的選項物件，例如：`await axios.post(`${base}/chat/completions`, payload, { headers, timeout: 30000, httpsAgent });`"
     }
 ]

.gitea/ai-review/findings.json

@@ -1 +1,37 @@
-[]
+[
+  {
+    "level": "warning",
+    "role": "Aria",
+    "location": "app/preflight.test.js:25",
+    "suggestion": "測試描述使用英文。請確保專案在測試描述的語言上保持一致性。如果專案主要使用繁體中文（如 `app/preflight.js` 中的 JSDoc 和日誌），則應將此測試描述翻譯為繁體中文。",
+    "is_new": true
+  },
+  {
+    "level": "info",
+    "role": "Aria",
+    "location": "app/preflight.test.js:1-4",
+    "suggestion": "匯入語句的排序不一致。建議遵循一致的排序規則，例如：內建模組、第三方模組、本地模組，並在各組內按字母順序排序。",
+    "is_new": true
+  },
+  {
+    "level": "info",
+    "role": "Aria",
+    "location": "app/preflight.test.js:7-12",
+    "suggestion": "此陣列字面量較長。雖然已分行，但可以考慮將每個元素獨立一行並保持一致的縮排，以提高可讀性。",
+    "is_new": true
+  },
+  {
+    "level": "info",
+    "role": "Aria",
+    "location": "app/preflight.test.js:14",
+    "suggestion": "函數名稱 `clearLLMEnv` 雖然可理解，但可以更具描述性，例如 `clearLlmEnvironmentVariables` 或 `resetLlmEnv`。",
+    "is_new": true
+  },
+  {
+    "level": "info",
+    "role": "Aria",
+    "location": "app/preflight.test.js:149",
+    "suggestion": "此單行註解風格與其他部分可能不一致。建議遵循專案統一的註解風格指南。",
+    "is_new": true
+  }
+]

app/git.js

@@ -62,11 +62,22 @@ function withAskpass(workspace, fn) {
   const askpassScript = path.join(workspace, '.git-askpass.sh');
   fs.writeFileSync(askpassScript, '#!/bin/sh\necho "$GIT_TOKEN"\n', { mode: 0o700 });
   const credEnv = { ...process.env, GIT_ASKPASS: askpassScript, GIT_USERNAME: 'x-token', GIT_TOKEN: GITEA_TOKEN };
+  const cleanup = () => { try { fs.unlinkSync(askpassScript); } catch {} };
+  let result;
   try {
-    return fn(credEnv);
-  } finally {
-    try { fs.unlinkSync(askpassScript); } catch {}
+    result = fn(credEnv);
+  } catch (e) {
+    cleanup();
+    throw e;
   }
+  // Defer cleanup until an async callback settles, otherwise the askpass script
+  // is deleted at the first `await` and later network ops (e.g. git push) fail
+  // with "cannot exec .git-askpass.sh". Sync callbacks clean up immediately.
+  if (result && typeof result.then === 'function') {
+    return result.finally(cleanup);
+  }
+  cleanup();
+  return result;
 }
 
 function readGitOutput(run, args, cwd, env) {

app/git.test.js

@@ -93,6 +93,18 @@ describe('commitAndPush', () => {
     }
   });
 
+  it('keeps the askpass script present while the network push runs', async () => {
+    let askpassExistsAtPush = null;
+    const spawn = makeSpawn({
+      push: (_args, opts) => {
+        askpassExistsAtPush = !!(opts?.env?.GIT_ASKPASS && fs.existsSync(opts.env.GIT_ASKPASS));
+        return { status: 0, stdout: '', stderr: '', error: null };
+      },
+    });
+    await commitAndPush(workspace, path.join(workspace, 'repo'), spawn, sourceRoot);
+    assert.equal(askpassExistsAtPush, true, 'askpass script must still exist when git push runs');
+  });
+
   it('cleans up askpass script after successful run', async () => {
     await commitAndPush(workspace, path.join(workspace, 'repo'), makeSpawn(), sourceRoot);
     const leftover = fs.readdirSync(workspace).filter(f => f.endsWith('.git-askpass.sh'));

app/preflight.js

@@ -93,24 +93,31 @@ export async function verifyLLM() {
  * 集中執行所有驗證相關設定的前置檢查；全部通過回傳 true，任一失敗回傳 false。
  * 僅做唯讀的認證/連線確認，不發布任何 comment。
  */
-export async function runPreflight(workspace = process.env.GITHUB_WORKSPACE || '/workspace') {
+export async function runPreflight(workspace = process.env.GITHUB_WORKSPACE || '/workspace', deps = {}) {
+  const {
+    checkEnv = checkRequiredEnv,
+    verifyToken = verifyGiteaToken,
+    verifyComment = verifyCommentToken,
+    verifyRemote = verifyRemoteAccess,
+    verifyLLMFn = verifyLLM,
+  } = deps;
   step('Step1.5', '前置驗證（驗證相關設定）');
 
-  const env = checkRequiredEnv();
+  const env = checkEnv();
   if (!env.ok) {
     error(`缺少必要環境變數: ${env.missing.join(', ')}`);
     return false;
   }
   ok('必要環境變數齊全 (GITEA_TOKEN, GITEA_REPOSITORY, PR_NUMBER)');
 
-  const gitea = await verifyGiteaToken();
+  const gitea = await verifyToken();
   if (!gitea.ok) {
     error(`GITEA_TOKEN 驗證失敗（無法讀取 repo ${GITEA_REPOSITORY}）: ${gitea.error}`);
     return false;
   }
   ok(`GITEA_TOKEN 可讀取 repo ${GITEA_REPOSITORY}`);
 
-  const comment = await verifyCommentToken();
+  const comment = await verifyComment();
   if (!comment.ok) {
     error(`GITEA_COMMENT_TOKEN 驗證失敗: ${comment.error}`);
     return false;
@@ -118,14 +125,14 @@ export async function runPreflight(workspace = process.env.GITHUB_WORKSPACE || '
   if (comment.skipped) line('未提供 GITEA_COMMENT_TOKEN，comment 將沿用 GITEA_TOKEN');
   else ok('GITEA_COMMENT_TOKEN 可用');
 
-  const remote = verifyRemoteAccess(workspace);
+  const remote = verifyRemote(workspace);
   if (!remote.ok) {
     error(`git push 認證/連線驗證失敗（ls-remote）: ${remote.error}`);
     return false;
   }
   ok('git remote 認證可用（ls-remote 成功）');
 
-  const llm = await verifyLLM();
+  const llm = await verifyLLMFn();
   if (!llm.ok) {
     error(`LLM 驗證失敗: ${llm.error}`);
     return false;

app/preflight.test.js

@@ -188,10 +188,76 @@ describe('verifyLLM', () => {
 });
 
 describe('runPreflight', () => {
+  // Stub deps that all succeed; individual tests override one to fail.
+  function makeDeps(overrides = {}) {
+    return {
+      checkEnv: () => ({ ok: true, missing: [] }),
+      verifyToken: async () => ({ ok: true }),
+      verifyComment: async () => ({ ok: true }),
+      verifyRemote: () => ({ ok: true }),
+      verifyLLMFn: async () => ({ ok: true, provider: 'openai', keyIndex: 1, total: 1 }),
+      ...overrides,
+    };
+  }
+
   it('returns false and stops early when required env is missing', async () => {
     // Config constants default to empty in the test environment, so the
     // required-env check fails before any network call is attempted.
     const result = await runPreflight();
     assert.equal(result, false);
   });
+
+  it('returns true when every verification step succeeds', async () => {
+    const result = await runPreflight('/ws', makeDeps());
+    assert.equal(result, true);
+  });
+
+  it('returns true when the comment token check is skipped', async () => {
+    const result = await runPreflight('/ws', makeDeps({
+      verifyComment: async () => ({ ok: true, skipped: true }),
+    }));
+    assert.equal(result, true);
+  });
+
+  it('returns false when the Gitea token check fails', async () => {
+    let remoteCalled = false;
+    const result = await runPreflight('/ws', makeDeps({
+      verifyToken: async () => ({ ok: false, error: 'HTTP 401' }),
+      verifyRemote: () => { remoteCalled = true; return { ok: true }; },
+    }));
+    assert.equal(result, false);
+    assert.equal(remoteCalled, false, 'should stop before later checks');
+  });
+
+  it('returns false when the comment token check fails', async () => {
+    const result = await runPreflight('/ws', makeDeps({
+      verifyComment: async () => ({ ok: false, error: 'HTTP 401' }),
+    }));
+    assert.equal(result, false);
+  });
+
+  it('returns false when git remote access fails', async () => {
+    let llmCalled = false;
+    const result = await runPreflight('/ws', makeDeps({
+      verifyRemote: () => ({ ok: false, error: 'auth failed' }),
+      verifyLLMFn: async () => { llmCalled = true; return { ok: true }; },
+    }));
+    assert.equal(result, false);
+    assert.equal(llmCalled, false, 'should stop before the LLM check');
+  });
+
+  it('returns false when LLM verification fails', async () => {
+    const result = await runPreflight('/ws', makeDeps({
+      verifyLLMFn: async () => ({ ok: false, error: '所有 key 驗證失敗' }),
+    }));
+    assert.equal(result, false);
+  });
+
+  it('passes the workspace through to the remote-access check', async () => {
+    let captured;
+    await runPreflight('/custom/ws', makeDeps({
+      verifyRemote: (ws) => { captured = ws; return { ok: true }; },
+    }));
+    assert.equal(captured, '/custom/ws');
+  });
 });