From cef674d7ad28428f50a92698b4e37558c345165e Mon Sep 17 00:00:00 2001 From: HugoXK Date: Wed, 4 Nov 2026 14:35:33 +0800 Subject: [PATCH 1/2] feat: verify branch protection before auto-merge Read the branch protection rule, verify required status checks and approvals, support squash/merge/rebase, expose protection state in settings, and block auto-merge when protection is unavailable. --- changelog.d/merge-policy.security.md | 1 + src/app.ts | 18 ++++++++- src/jobs/service.ts | 24 +++++++++++- src/repos/github.ts | 18 +++++++++ src/views/pages.ts | 5 +++ test/jobs.test.ts | 58 ++++++++++++++++++++++++++++ 6 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 changelog.d/merge-policy.security.md diff --git a/changelog.d/merge-policy.security.md b/changelog.d/merge-policy.security.md new file mode 100644 index 0000000..22e755e --- /dev/null +++ b/changelog.d/merge-policy.security.md @@ -0,0 +1 @@ +Added branch protection verification to auto-merge. Fledge reads the target branch protection rule, verifies required status checks and required approvals, supports squash/merge/rebase, and refuses to auto-merge when protection cannot be verified or a required gate is missing. diff --git a/src/app.ts b/src/app.ts index 870ad41..14c798e 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1510,6 +1510,18 @@ export async function buildApp(options: BuildOptions): Promise { const downgraded = currentRole === 'maintainer' && !allowed.includes('maintainer') const nextRole = downgraded ? 'developer' : repo.repo_role const enableReviewer = nextRole === 'maintainer' && prepared.canReview && repo.reviewer_enabled === 1 + let protectionState = repo.protection_state + let protectionCheckedAt = repo.protection_checked_at + const protectionBranch = repo.target_branch !== '' ? repo.target_branch : (prepared.upstream.defaultBranch || repo.default_branch) + try { + const protection = await new GithubClient(token, config.github.apiBase).getBranchProtection(prepared.upstream.owner, prepared.upstream.name, protectionBranch) + protectionState = protection === null ? 'unknown' : protection.protected ? 'protected' : 'unprotected' + protectionCheckedAt = nowIso() + } catch (error) { + protectionState = 'unknown' + protectionCheckedAt = nowIso() + log.warn('branch protection check failed', { repo: repo.id, error: String(error) }) + } store.updateRepo(repo.id, { upstream_owner: prepared.upstream.owner, upstream_name: prepared.upstream.name, @@ -1523,6 +1535,8 @@ export async function buildApp(options: BuildOptions): Promise { repo_role: nextRole, auto_merge: downgraded ? 0 : repo.auto_merge, role_checked_at: nowIso(), + protection_state: protectionState, + protection_checked_at: protectionCheckedAt, can_review: prepared.canReview ? 1 : 0, can_create_issue: prepared.canCreateIssue ? 1 : 0, reviewer_enabled: enableReviewer ? 1 : 0, @@ -1630,6 +1644,8 @@ export async function buildApp(options: BuildOptions): Promise { plan_required: body.planRequired === '1' ? 1 : 0, allow_paths: (body.allowPaths ?? '').trim().slice(0, 2000), deny_paths: (body.denyPaths ?? '').trim().slice(0, 2000), + merge_method: body.mergeMethod === 'merge' || body.mergeMethod === 'rebase' ? body.mergeMethod : 'squash', + require_protection: body.requireProtection === '1' ? 1 : 0, auto_iterate: body.auto === '1' ? 1 : 0, enabled: body.enabled === '1' ? 1 : 0, interval_minutes: interval, @@ -1644,7 +1660,7 @@ export async function buildApp(options: BuildOptions): Promise { log.warn('team playbook not updated after goal change', { repo: repo.id, error: String(error) }) }) } - store.audit(viewer.user.id, 'repo.settings', `${repo.owner}/${repo.name} role=${repoRole}${roleChanged ? ' role=changed' : ''} autoMerge=${canMaintain && body.autoMerge === '1' ? 'on' : 'off'} planRequired=${body.planRequired === '1' ? 'on' : 'off'} reviewer=${canMaintain && repo.can_review === 1 && body.review === '1' ? 'on' : 'off'}${goalChanged ? ' goal=changed' : ''} base=${targetBranch}`) + store.audit(viewer.user.id, 'repo.settings', `${repo.owner}/${repo.name} role=${repoRole}${roleChanged ? ' role=changed' : ''} autoMerge=${canMaintain && body.autoMerge === '1' ? 'on' : 'off'} planRequired=${body.planRequired === '1' ? 'on' : 'off'} merge=${body.mergeMethod ?? 'squash'} protection=${body.requireProtection === '1' ? 'required' : 'optional'} reviewer=${canMaintain && repo.can_review === 1 && body.review === '1' ? 'on' : 'off'}${goalChanged ? ' goal=changed' : ''} base=${targetBranch}`) return reply.redirect(`/repos/${repo.id}?notice=${encodeURIComponent(goalChanged ? '设置已保存；团队规则已按新目标更新。' : '设置已保存。')}#repo-settings`) }) diff --git a/src/jobs/service.ts b/src/jobs/service.ts index e40c088..03b272d 100644 --- a/src/jobs/service.ts +++ b/src/jobs/service.ts @@ -559,7 +559,29 @@ export class Jobs { this.store.audit(job.user_id, 'job.auto_merge_skipped', `${job.id}: changes requested`) return } - const merged = await client.mergePullRequest(owner, name, number, { sha: pr.headSha, mergeMethod: 'squash' }) + if (repo.require_protection === 1) { + const protection = await client.getBranchProtection(owner, name, pr.base) + if (protection === null || !protection.protected) { + this.store.audit(job.user_id, 'job.auto_merge_skipped', `${job.id}: branch protection unavailable`) + return + } + if (protection.requiredChecks.length > 0) { + const passing = new Set(checks.filter((run) => run.status === 'completed' && ['success', 'neutral', 'skipped'].includes(run.conclusion ?? '')).map((run) => run.name)) + if (!protection.requiredChecks.every((context) => passing.has(context))) { + this.store.audit(job.user_id, 'job.auto_merge_skipped', `${job.id}: required checks missing`) + return + } + } + if (protection.requiredApprovals > 0) { + const approvals = [...latest.values()].filter((state) => state === 'APPROVED').length + if (approvals < protection.requiredApprovals) { + this.store.audit(job.user_id, 'job.auto_merge_skipped', `${job.id}: required approvals missing`) + return + } + } + } + const requestedMethod = repo.merge_method === 'merge' || repo.merge_method === 'rebase' ? repo.merge_method : 'squash' + const merged = await client.mergePullRequest(owner, name, number, { sha: pr.headSha, mergeMethod: requestedMethod }) if (merged.merged) { const workItem = job.work_item_id === null ? undefined : this.store.getWorkItem(job.work_item_id) if (workItem !== undefined) { diff --git a/src/repos/github.ts b/src/repos/github.ts index 1225559..4fdead0 100644 --- a/src/repos/github.ts +++ b/src/repos/github.ts @@ -324,6 +324,24 @@ export class GithubClient { : [] } + async getBranchProtection(owner: string, name: string, branch: string): Promise<{ protected: boolean; requiredChecks: string[]; requiredApprovals: number } | null> { + try { + const result = await this.request<{ + required_status_checks?: { contexts?: string[] } | null + required_pull_request_reviews?: { required_approving_review_count?: number } | null + }>('GET', `/repos/${encodeURIComponent(owner)}/${encodeURIComponent(name)}/branches/${encodeURIComponent(branch)}/protection`) + return { + protected: true, + requiredChecks: Array.isArray(result.required_status_checks?.contexts) ? result.required_status_checks.contexts : [], + requiredApprovals: Number(result.required_pull_request_reviews?.required_approving_review_count ?? 0), + } + } catch (error) { + if (error instanceof GithubError && error.status === 404) return { protected: false, requiredChecks: [], requiredApprovals: 0 } + if (error instanceof GithubError && error.status === 403) return null + throw error + } + } + async mergePullRequest(owner: string, name: string, number: number, input: { sha: string; mergeMethod?: 'merge' | 'squash' | 'rebase' }): Promise<{ merged: boolean; sha?: string; message?: string }> { const result = await this.request<{ merged?: boolean; sha?: string; message?: string }>( 'PUT', diff --git a/src/views/pages.ts b/src/views/pages.ts index 216d57a..ea1592e 100644 --- a/src/views/pages.ts +++ b/src/views/pages.ts @@ -674,6 +674,11 @@ export function repoPage(input: { config: Config; locale: Locale; viewer: Viewer +
+ + +
+

${escapeHtml(t('Branch protection', 'Branch protection'))}: ${escapeHtml(repo.protection_state ?? 'unknown')}${repo.protection_checked_at === null ? '' : ` · ${escapeHtml(humanTimeShort(repo.protection_checked_at))}`}

${roleMissing ? `

${escapeHtml(t('先选择你的角色，审核和自动合并才会生效。', 'Choose your role before review or auto-merge can take effect.'))}

` : !githubCanReview ? `

${escapeHtml(t('你在这个上游没有 admin/maintain 权限，审核角色不可用。', 'You do not have admin/maintain permission on this upstream, so the reviewer role is unavailable.'))}

` : !canMaintain ? `

${escapeHtml(t('developer 不能启用 PR 审核或自动合并；切换为 maintainer 后才可用。', 'A developer cannot enable PR review or auto-merge; switch to maintainer first.'))}

` : ''} ${advanced ? `
${escapeHtml(t('团队与调度规则', 'Team and scheduling rules'))}

${escapeHtml(t('保存目标后，团队 playbook 会按新目标更新：总控 → 负责人 → 产品/测试/安全/审核/开发。审核角色只有勾选“启用 PR 审核”后才会参与。', 'Saving the goal updates the team playbook: Chief -> Lead -> Product/Test/Security/Reviewer/Dev. The Reviewer takes part only when "Enable PR reviewer" is checked.'))}

diff --git a/test/jobs.test.ts b/test/jobs.test.ts index 4e070e4..d65e46c 100644 --- a/test/jobs.test.ts +++ b/test/jobs.test.ts @@ -548,6 +548,10 @@ test('maintainer auto-merge merges only after checks pass and no changes are req json(response, 200, reviews) return } + if (method === 'GET' && url === '/repos/o/r/branches/main/protection') { + json(response, 200, { required_status_checks: { contexts: ['ci'] }, required_pull_request_reviews: { required_approving_review_count: 0 } }) + return + } if (method === 'PUT' && url === '/repos/o/r/pulls/1/merge') { merges.push('merge') json(response, 200, { merged: true, sha: 'def' }) @@ -759,3 +763,57 @@ test('a running job can be cancelled and a paused job can be resumed', async () store.close() } }) + +test('auto-merge is skipped when branch protection is unavailable', async () => { + const merges: string[] = [] + const github = await startFakeServer((request, response) => { + const method = request.method ?? 'GET' + const url = request.url ?? '' + if (method === 'GET' && url === '/repos/o/r/pulls/4') { + json(response, 200, { number: 4, html_url: 'https://github.com/o/r/pull/4', title: 'fix', state: 'open', head: { ref: 'fledge/fix', sha: 'abc' }, base: { ref: 'main' }, mergeable: true, mergeable_state: 'clean', draft: false, merged: false }) + return + } + if (method === 'GET' && url === '/repos/o/r/commits/abc/check-runs?per_page=100') { + json(response, 200, { check_runs: [{ name: 'ci', status: 'completed', conclusion: 'success' }] }) + return + } + if (method === 'GET' && url === '/repos/o/r/commits/abc/status') { + json(response, 200, { state: 'success', statuses: [] }) + return + } + if (method === 'GET' && url === '/repos/o/r/pulls/4/reviews?per_page=100') { + json(response, 200, []) + return + } + if (method === 'GET' && url === '/repos/o/r/branches/main/protection') { + json(response, 404, { message: 'Not Found' }) + return + } + if (method === 'PUT' && url === '/repos/o/r/pulls/4/merge') { + merges.push('merge') + json(response, 200, { merged: true }) + return + } + if (method === 'POST' && url === '/repos/o/r/issues/4/comments') { + json(response, 200, {}) + return + } + json(response, 404, { message: 'Not Found' }) + }) + const base = testConfig() + const config = testConfig({ github: { ...base.github, apiBase: github.url } }) + const store = new Store(':memory:') + const jobs = new Jobs(store, config, { name: 'noop', async run() { return { status: 'running' } } }, new MemoryMailer()) + try { + const user = makeUser(store, { login: 'auto-merge-protection-user', status: 'approved' }) + const repo = store.createRepo({ id: 'repo_auto_merge_protection', userId: user.id, owner: 'o', name: 'r', defaultBranch: 'main', tokenEnc: encryptSecret('gh-token', config.tokenEncryptionKey), permission: 'admin', repoRole: 'maintainer', autoMerge: true, requireProtection: true, goal: '', autoIterate: false, intervalMinutes: 1440 }) + const secret = 'auto-merge-protection-secret' + store.createJob({ id: 'job_auto_merge_protection', repoId: repo.id, userId: user.id, trigger: 'manual', goal: 'g', callbackHash: sha256Hex(secret), platformTokenHash: '', kind: 'iterate', baseBranch: 'main' }) + store.updateJob('job_auto_merge_protection', { status: 'running', started_at: new Date().toISOString() }) + await jobs.handleCallback('job_auto_merge_protection', secret, { status: 'succeeded', prUrl: 'https://github.com/o/r/pull/4', summary: 'done' }) + assert.equal(merges.length, 0) + } finally { + store.close() + await github.close() + } +}) From f34a585139b44f49664294b86ae78382e08b1db9 Mon Sep 17 00:00:00 2001 From: HugoXK Date: Wed, 4 Nov 2026 15:44:44 +0800 Subject: [PATCH 2/2] fix: remove useless protection state assignments Read the branch protection result once so CodeQL no longer reports unused initial values. --- src/app.ts | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/app.ts b/src/app.ts index 14c798e..64b5947 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1510,18 +1510,15 @@ export async function buildApp(options: BuildOptions): Promise { const downgraded = currentRole === 'maintainer' && !allowed.includes('maintainer') const nextRole = downgraded ? 'developer' : repo.repo_role const enableReviewer = nextRole === 'maintainer' && prepared.canReview && repo.reviewer_enabled === 1 - let protectionState = repo.protection_state - let protectionCheckedAt = repo.protection_checked_at const protectionBranch = repo.target_branch !== '' ? repo.target_branch : (prepared.upstream.defaultBranch || repo.default_branch) - try { - const protection = await new GithubClient(token, config.github.apiBase).getBranchProtection(prepared.upstream.owner, prepared.upstream.name, protectionBranch) - protectionState = protection === null ? 'unknown' : protection.protected ? 'protected' : 'unprotected' - protectionCheckedAt = nowIso() - } catch (error) { - protectionState = 'unknown' - protectionCheckedAt = nowIso() - log.warn('branch protection check failed', { repo: repo.id, error: String(error) }) - } + const protection = await new GithubClient(token, config.github.apiBase) + .getBranchProtection(prepared.upstream.owner, prepared.upstream.name, protectionBranch) + .catch((error) => { + log.warn('branch protection check failed', { repo: repo.id, error: String(error) }) + return null + }) + const protectionState = protection === null ? 'unknown' : protection.protected ? 'protected' : 'unprotected' + const protectionCheckedAt = nowIso() store.updateRepo(repo.id, { upstream_owner: prepared.upstream.owner, upstream_name: prepared.upstream.name,