diff --git a/app/git.js b/app/git.js index c9bc837..143e848 100644 --- a/app/git.js +++ b/app/git.js @@ -3,29 +3,39 @@ import fs from 'fs'; import path from 'path'; import { GITEA_SERVER_URL, GITEA_REPOSITORY, GITEA_TOKEN, PR_HEAD_BRANCH, FINDINGS_PATH } from './config.js'; -function git(args, cwd) { - const result = spawnSync('git', args, { cwd, encoding: 'utf8' }); - if (result.error) throw result.error; - if (result.status !== 0) throw new Error((result.stderr || result.stdout || '').trim()); - return (result.stdout || '').trim(); +function makeRunner(spawn) { + return function run(args, cwd, env) { + const opts = { cwd, encoding: 'utf8' }; + if (env) opts.env = env; + const result = spawn('git', args, opts); + if (result.error) throw result.error; + if (result.status !== 0) throw new Error((result.stderr || result.stdout || '').trim()); + return (result.stdout || '').trim(); + }; } -export async function commitAndPush(workspace) { - const remoteUrl = GITEA_SERVER_URL.replace(/\/$/, '') - .replace('https://', `https://${GITEA_TOKEN}@`) - .replace('http://', `http://${GITEA_TOKEN}@`) + `/${GITEA_REPOSITORY}.git`; +export async function commitAndPush(workspace, _spawnSync = spawnSync) { + const run = makeRunner(_spawnSync); + const baseUrl = GITEA_SERVER_URL.replace(/\/$/, ''); + const remoteUrl = `${baseUrl}/${GITEA_REPOSITORY}.git`; const repoDir = path.join(workspace, 'repo'); + // Write a temporary askpass script so the token never appears in the URL or process list + const askpassScript = path.join(workspace, '.git-askpass.sh'); + fs.writeFileSync(askpassScript, `#!/bin/sh\necho "${GITEA_TOKEN}"\n`, { mode: 0o700 }); + + const credEnv = { ...process.env, GIT_ASKPASS: askpassScript, GIT_USERNAME: 'x-token' }; + try { if (!fs.existsSync(repoDir)) { - git(['clone', '--depth=1', '--branch', PR_HEAD_BRANCH, remoteUrl, repoDir], workspace); + run(['clone', '--depth=1', '--branch', PR_HEAD_BRANCH, remoteUrl, repoDir], workspace, credEnv); } - git(['config', 'user.email', 'ai-review[bot]@gitea'], repoDir); - git(['config', 'user.name', 'AI Review Bot'], repoDir); - git(['fetch', 'origin', PR_HEAD_BRANCH], repoDir); - git(['checkout', PR_HEAD_BRANCH], repoDir); + run(['config', 'user.email', 'ai-review[bot]@gitea'], repoDir); + run(['config', 'user.name', 'AI Review Bot'], repoDir); + run(['fetch', 'origin', PR_HEAD_BRANCH], repoDir, credEnv); + run(['checkout', PR_HEAD_BRANCH], repoDir); // 將 findings.json 從 workspace 複製到 clone 的 repo const srcFindings = path.join(workspace, FINDINGS_PATH); @@ -33,19 +43,21 @@ export async function commitAndPush(workspace) { fs.mkdirSync(path.dirname(destFindings), { recursive: true }); fs.copyFileSync(srcFindings, destFindings); - git(['add', FINDINGS_PATH], repoDir); + run(['add', FINDINGS_PATH], repoDir); - const status = git(['status', '--porcelain'], repoDir); + const status = run(['status', '--porcelain'], repoDir); if (!status) { console.log(' findings.json 無變更,跳過 commit'); return; } - const out = git(['commit', '-m', 'chore: update ai-review findings [skip ci]'], repoDir); + const out = run(['commit', '-m', 'chore: update ai-review findings [skip ci]'], repoDir); const commitHash = out.match(/\[.+ ([a-f0-9]+)\]/)?.[1] || 'unknown'; - git(['push', remoteUrl, PR_HEAD_BRANCH], repoDir); + run(['push', remoteUrl, PR_HEAD_BRANCH], repoDir, credEnv); console.log(` ✅ persisted findings commit=${commitHash} push=${PR_HEAD_BRANCH}`); } catch (e) { console.log(` ⚠️ Runner failed: commit/push 失敗: ${e.message}`); + } finally { + try { fs.unlinkSync(askpassScript); } catch {} } } diff --git a/app/git.test.js b/app/git.test.js new file mode 100644 index 0000000..d96efce --- /dev/null +++ b/app/git.test.js @@ -0,0 +1,93 @@ +import { describe, it, before, after, beforeEach } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { commitAndPush } from './git.js'; + +// --- helpers --- +function makeTmpWorkspace() { + const ws = fs.mkdtempSync(path.join(os.tmpdir(), 'git-test-')); + // Pre-create repo dir so clone branch is skipped + fs.mkdirSync(path.join(ws, 'repo'), { recursive: true }); + // Create a findings.json to copy + const findingsDir = path.join(ws, '.gitea/ai-review'); + fs.mkdirSync(findingsDir, { recursive: true }); + fs.writeFileSync(path.join(findingsDir, 'findings.json'), '[]'); + return ws; +} + +// Default stub: all commands succeed, status returns changes +function makeSpawn(overrides = {}) { + const calls = []; + const spawn = (cmd, args, opts) => { + const key = args[0]; + calls.push({ cmd, args, opts }); + if (overrides[key]) return overrides[key](args, opts); + if (key === 'status') return { status: 0, stdout: 'M .gitea/ai-review/findings.json', stderr: '', error: null }; + if (key === 'commit') return { status: 0, stdout: '[feature-branch abc1234] chore', stderr: '', error: null }; + return { status: 0, stdout: '', stderr: '', error: null }; + }; + spawn.calls = calls; + return spawn; +} + +describe('commitAndPush', () => { + let workspace; + + before(() => { workspace = makeTmpWorkspace(); }); + after(() => { fs.rmSync(workspace, { recursive: true, force: true }); }); + beforeEach(() => { + // Remove leftover askpass scripts between tests + for (const f of fs.readdirSync(workspace)) { + if (f.endsWith('.git-askpass.sh')) fs.unlinkSync(path.join(workspace, f)); + } + }); + + it('does not embed token in any git command argument', async () => { + const spawn = makeSpawn(); + await commitAndPush(workspace, spawn); + + for (const { args } of spawn.calls) { + assert.ok(!args.join(' ').includes('test-token'), `Token leaked in git args: ${args.join(' ')}`); + } + }); + + it('uses GIT_ASKPASS env for network operations (fetch, push, clone)', async () => { + const spawn = makeSpawn(); + await commitAndPush(workspace, spawn); + + const networkOps = ['fetch', 'push', 'clone']; + const networkCalls = spawn.calls.filter(c => networkOps.includes(c.args[0])); + assert.ok(networkCalls.length > 0, 'expected at least one network git call'); + + for (const { args, opts } of networkCalls) { + assert.ok(opts?.env?.GIT_ASKPASS, `GIT_ASKPASS missing for git ${args[0]}`); + } + }); + + it('cleans up askpass script after successful run', async () => { + await commitAndPush(workspace, makeSpawn()); + const leftover = fs.readdirSync(workspace).filter(f => f.endsWith('.git-askpass.sh')); + assert.equal(leftover.length, 0, 'askpass script was not cleaned up'); + }); + + it('cleans up askpass script even when git fails', async () => { + const failSpawn = () => ({ status: 1, stdout: '', stderr: 'fatal: error', error: null }); + await commitAndPush(workspace, failSpawn); + const leftover = fs.readdirSync(workspace).filter(f => f.endsWith('.git-askpass.sh')); + assert.equal(leftover.length, 0, 'askpass script was not cleaned up after failure'); + }); + + it('skips commit when status shows no changes', async () => { + const spawn = makeSpawn({ status: () => ({ status: 0, stdout: '', stderr: '', error: null }) }); + await commitAndPush(workspace, spawn); + const commitCalled = spawn.calls.some(c => c.args[0] === 'commit'); + assert.equal(commitCalled, false, 'commit should not run when there are no changes'); + }); + + it('does not throw when git command fails', async () => { + const failSpawn = () => ({ status: 1, stdout: '', stderr: 'fatal: error', error: null }); + await assert.doesNotReject(() => commitAndPush(workspace, failSpawn)); + }); +}); diff --git a/app/package.json b/app/package.json index b5e3877..466e7c7 100644 --- a/app/package.json +++ b/app/package.json @@ -2,6 +2,9 @@ "name": "ai-code-review", "version": "1.0.0", "type": "module", + "scripts": { + "test": "node --test app/git.test.js" + }, "dependencies": { "axios": "^1.6.7", "js-yaml": "^4.1.0",