CodeIssuesPull RequestsActionsSecurityInsights
✨ AI
More
Settings

feat(protection): branch protection is enforced at push time — the checkboxes stop lying #5546

MergedXSccantynz wants to mergefeat/enforce-branch-protectionmainopened 6d ago
6 changed files+338−9
Addeddrizzle/0130_reset_unenforced_require_pr.sql+23−0View fileUnifiedSplit
1-- Reset require_pull_request before enforcement lands (2026-08-27).
2--
3-- Until today, branch_protection.require_pull_request was written by
4-- repo-bootstrap (default TRUE on every new repo's default branch),
5-- rendered as a checked box on the gates page — and read by NOTHING in
6-- the push path. Every row holding TRUE is a claim nobody ever agreed
7-- to have enforced: the flag predates its own meaning.
8--
9-- The same deploy that ships this migration starts enforcing the flag
10-- for real (push-policy.ts). Leaving the rows TRUE would instantly
11-- reject every solo developer's `git push origin main` on repos whose
12-- owners never chose that behavior — including this platform's own
13-- deploy-by-push flow. So: one honest reset to FALSE everywhere.
14-- Owners who want required-PR protection flip it back on deliberately,
15-- and from today the checkbox does what it says.
16--
17-- allow_force_push / allow_deletion keep their FALSE defaults and are
18-- NOT reset: enforcing those blocks only history rewrites and branch
19-- deletion on protected branches — behavior every existing row's owner
20-- plausibly wanted, and which breaks no normal push.
21
22--> statement-breakpoint
23UPDATE "branch_protection" SET "require_pull_request" = false;
Addedsrc/__tests__/branch-protection-push-enforcement.test.ts+184−0View fileUnifiedSplit
1/**
2 * Push-time branch-protection enforcement (2026-08-27).
3 *
4 * Until this change, branch_protection.require_pull_request /
5 * allow_force_push / allow_deletion were written by repo-bootstrap,
6 * rendered as configured on the gates page — and read by NOTHING in the
7 * push path. `git push --force origin main` sailed through "protected"
8 * branches. The flow audit ranked it the platform's headline silent
9 * trust failure.
10 *
11 * Covered here:
12 * - evaluatePushPolicy blocks protected-branch deletion unless
13 * allowDeletion, blocks direct updates when requirePullRequest, and
14 * always allows branch CREATION (a fresh repo's first
15 * `git push -u origin main` must never be rejected).
16 * - fail-open when matchProtection throws.
17 * - the pre-receive hook materializes the no-force-push branch list and
18 * the merge-base ancestry check (force-push needs the quarantined
19 * objects, so it cannot be decided at the policy layer).
20 *
21 * matchProtection is stubbed via mock.module (process-global — restored
22 * in afterAll, same discipline as the other suites).
23 */
24
25import { describe, it, expect, mock, afterAll } from "bun:test";
26
27const _real_bp = await import("../lib/branch-protection");
28
29type FakeRule = {
30 pattern: string;
31 requirePullRequest: boolean;
32 allowForcePush: boolean;
33 allowDeletion: boolean;
34} | null;
35
36let _ruleFor: (branch: string) => FakeRule | Promise<FakeRule> = () => null;
37
38mock.module("../lib/branch-protection", () => ({
39 ..._real_bp,
40 matchProtection: async (_repoId: string, branch: string) =>
41 await _ruleFor(branch),
42}));
43
44afterAll(() => {
45 mock.module("../lib/branch-protection", () => _real_bp);
46});
47
48const { evaluatePushPolicy, installPackInspectionHook, ZERO_SHA } =
49 await import("../lib/push-policy");
50
51const SHA_A = "a".repeat(40);
52const SHA_B = "b".repeat(40);
53
54const protectedMain: FakeRule = {
55 pattern: "main",
56 requirePullRequest: false,
57 allowForcePush: false,
58 allowDeletion: false,
59};
60
61describe("evaluatePushPolicy — branch protection", () => {
62 it("blocks deleting a protected branch when allowDeletion=false", async () => {
63 _ruleFor = (b) => (b === "main" ? protectedMain : null);
64 const r = await evaluatePushPolicy({
65 repositoryId: "repo-1",
66 refs: [{ oldSha: SHA_A, newSha: ZERO_SHA, refName: "refs/heads/main" }],
67 pusherUserId: "u1",
68 });
69 expect(r.allowed).toBe(false);
70 expect(r.violations.join(" ")).toContain("deletion is not allowed");
71 });
72
73 it("allows deleting when allowDeletion=true", async () => {
74 _ruleFor = () => ({ ...protectedMain!, allowDeletion: true });
75 const r = await evaluatePushPolicy({
76 repositoryId: "repo-1",
77 refs: [{ oldSha: SHA_A, newSha: ZERO_SHA, refName: "refs/heads/main" }],
78 pusherUserId: "u1",
79 });
80 expect(r.allowed).toBe(true);
81 });
82
83 it("blocks a direct update when requirePullRequest=true", async () => {
84 _ruleFor = () => ({ ...protectedMain!, requirePullRequest: true });
85 const r = await evaluatePushPolicy({
86 repositoryId: "repo-1",
87 refs: [{ oldSha: SHA_A, newSha: SHA_B, refName: "refs/heads/main" }],
88 pusherUserId: "u1",
89 });
90 expect(r.allowed).toBe(false);
91 expect(r.violations.join(" ")).toContain("open a pull request");
92 });
93
94 it("ALWAYS allows branch creation — the first push of a fresh repo", async () => {
95 _ruleFor = () => ({ ...protectedMain!, requirePullRequest: true });
96 const r = await evaluatePushPolicy({
97 repositoryId: "repo-1",
98 refs: [{ oldSha: ZERO_SHA, newSha: SHA_A, refName: "refs/heads/main" }],
99 pusherUserId: "u1",
100 });
101 expect(r.allowed).toBe(true);
102 });
103
104 it("allows direct updates when requirePullRequest=false (the new default)", async () => {
105 _ruleFor = () => protectedMain;
106 const r = await evaluatePushPolicy({
107 repositoryId: "repo-1",
108 refs: [{ oldSha: SHA_A, newSha: SHA_B, refName: "refs/heads/main" }],
109 pusherUserId: "u1",
110 });
111 expect(r.allowed).toBe(true);
112 });
113
114 it("ignores unprotected branches entirely", async () => {
115 _ruleFor = (b) => (b === "main" ? protectedMain : null);
116 const r = await evaluatePushPolicy({
117 repositoryId: "repo-1",
118 refs: [
119 { oldSha: SHA_A, newSha: ZERO_SHA, refName: "refs/heads/feature/x" },
120 ],
121 pusherUserId: "u1",
122 });
123 expect(r.allowed).toBe(true);
124 });
125
126 it("fails OPEN when matchProtection throws — a DB hiccup never wedges a push", async () => {
127 _ruleFor = () => {
128 throw new Error("db down");
129 };
130 const r = await evaluatePushPolicy({
131 repositoryId: "repo-1",
132 refs: [{ oldSha: SHA_A, newSha: ZERO_SHA, refName: "refs/heads/main" }],
133 pusherUserId: "u1",
134 });
135 expect(r.allowed).toBe(true);
136 });
137});
138
139describe("pre-receive hook — force-push denial plumbing", () => {
140 it("installs a hook whose script carries the merge-base ancestry check", async () => {
141 const hook = await installPackInspectionHook([], {
142 secretScan: false,
143 noForcePushBranches: ["main"],
144 });
145 expect(hook).not.toBeNull();
146 try {
147 const dir = hook!.env.GIT_CONFIG_VALUE_0;
148 const fs = await import("node:fs/promises");
149 const script = await fs.readFile(`${dir}/pre-receive`, "utf8");
150 expect(script).toContain("merge-base --is-ancestor");
151 expect(script).toContain("force pushes are not allowed");
152 const noff = await fs.readFile(`${dir}/no-force-push.txt`, "utf8");
153 expect(noff.trim()).toBe("main");
154 } finally {
155 await hook!.cleanup();
156 }
157 });
158
159 it("skips the hook entirely when there is nothing to enforce", async () => {
160 const hook = await installPackInspectionHook([], {
161 secretScan: false,
162 noForcePushBranches: [],
163 });
164 expect(hook).toBeNull();
165 });
166
167 it("drops branch names containing newlines — no file-format smuggling", async () => {
168 const hook = await installPackInspectionHook([], {
169 secretScan: false,
170 noForcePushBranches: ["ok-branch", "evil\ninjected"],
171 });
172 expect(hook).not.toBeNull();
173 try {
174 const dir = hook!.env.GIT_CONFIG_VALUE_0;
175 const fs = await import("node:fs/promises");
176 const noff = await fs.readFile(`${dir}/no-force-push.txt`, "utf8");
177 expect(noff).toContain("ok-branch");
178 expect(noff).not.toContain("evil");
179 expect(noff).not.toContain("injected");
180 } finally {
181 await hook!.cleanup();
182 }
183 });
184});
Modifiedsrc/lib/push-policy.ts+119−7View fileUnifiedSplit
4949 type PushContext,
5050} from "./rulesets";
5151import { config } from "./config";
52import { matchProtection } from "./branch-protection";
5253import type { RulesetRule, RepoRuleset } from "../db/schema";
5354import { jsonForScript } from "./json-for-script";
5455
222223 * pushed ref. Both file paths are embedded literals so the script is
223224 * fully self-contained.
224225 */
225function buildPreReceiveScript(evalScriptPath: string, rulesJsonPath: string): string {
226function buildPreReceiveScript(
227 evalScriptPath: string,
228 rulesJsonPath: string,
229 noForcePushPath?: string
230): string {
226231 const D = "$";
227232 return [
228233 "#!/bin/bash",
229234 "set -uo pipefail",
230235 `EVAL_SCRIPT='${evalScriptPath}'`,
231236 `RULES_JSON='${rulesJsonPath}'`,
237 `NOFF_FILE='${noForcePushPath ?? ""}'`,
232238 "FAILED=0",
233239 "",
234240 `while IFS=' ' read -r OLD NEW REF; do`,
235241 ` [[ "${D}NEW" =~ ^0+${D} ]] && continue`,
236242 "",
243 // Force-push denial for protected branches. This check must live HERE
244 // (not in evaluatePushPolicy) because it needs the pushed commit: inside
245 // git-receive-pack the quarantined objects are visible, so
246 // merge-base --is-ancestor can decide fast-forward vs rewrite. The
247 // branch names in NOFF_FILE were already resolved through
248 // matchProtection() in TypeScript — exact string compare only, so the
249 // glob semantics cannot drift between the two languages.
250 ` if [ -n "${D}NOFF_FILE" ] && [ -s "${D}NOFF_FILE" ] && [[ "${D}REF" == refs/heads/* ]] && ! [[ "${D}OLD" =~ ^0+${D} ]]; then`,
251 ` BRANCH="${D}{REF#refs/heads/}"`,
252 ` while IFS= read -r NOFF_BRANCH; do`,
253 ` [ -z "${D}NOFF_BRANCH" ] && continue`,
254 ` if [ "${D}BRANCH" = "${D}NOFF_BRANCH" ]; then`,
255 ` if ! git merge-base --is-ancestor "${D}OLD" "${D}NEW" 2>/dev/null; then`,
256 ` echo "remote: branch \\"${D}BRANCH\\" is protected; force pushes are not allowed" >&2`,
257 ` FAILED=1`,
258 ` fi`,
259 ` break`,
260 ` fi`,
261 ` done < "${D}NOFF_FILE"`,
262 ` fi`,
263 "",
237264 ` COMMITS_TMP=$(mktemp)`,
238265 ` SIZES_TMP=$(mktemp)`,
239266 ` PATHS_TMP=$(mktemp)`,
298325 */
299326export async function installPackInspectionHook(
300327 rulesets: ActiveRuleset[],
301 opts: { secretScan?: boolean } = {}
328 opts: { secretScan?: boolean; noForcePushBranches?: string[] } = {}
302329): Promise<{ env: Record<string, string>; cleanup: () => Promise<void> } | null> {
303330 const secretScan = opts.secretScan !== false;
331 const noForcePushBranches = (opts.noForcePushBranches ?? []).filter(
332 // Branch names go into a newline-delimited file the bash hook reads;
333 // a name containing a newline could smuggle in an extra entry.
334 (b) => b && !b.includes("\n")
335 );
304336
305337 // Collect pack-content rules from non-disabled rulesets.
306338 type RuleEntry = { rulesetName: string; enforcement: string; params: Record<string, unknown> };
319351 }
320352 }
321353
322 // No pack-content rules AND secret scan disabled → skip hook entirely.
323 if (!commitMsgRules.length && !blockedPathRules.length && !maxSizeRules.length && !secretScan) {
354 // No pack-content rules, no force-push denials, AND secret scan disabled
355 // → skip hook entirely.
356 if (
357 !commitMsgRules.length &&
358 !blockedPathRules.length &&
359 !maxSizeRules.length &&
360 !noForcePushBranches.length &&
361 !secretScan
362 ) {
324363 return null;
325364 }
326365
334373 );
335374 const evalScriptPath = join(dir, "eval.js");
336375 await writeFile(evalScriptPath, buildEvalScript(), { mode: 0o644 });
376 let noForcePushPath: string | undefined;
377 if (noForcePushBranches.length > 0) {
378 noForcePushPath = join(dir, "no-force-push.txt");
379 await writeFile(noForcePushPath, noForcePushBranches.join("\n") + "\n", {
380 mode: 0o644,
381 });
382 }
337383 const hookPath = join(dir, "pre-receive");
338 await writeFile(hookPath, buildPreReceiveScript(evalScriptPath, rulesJsonPath), { mode: 0o755 });
384 await writeFile(
385 hookPath,
386 buildPreReceiveScript(evalScriptPath, rulesJsonPath, noForcePushPath),
387 { mode: 0o755 }
388 );
339389 return {
340390 env: {
341391 GIT_CONFIG_COUNT: "1",
413463 );
414464 }
415465
466 // Branch protection — the parts decidable from the ref list alone.
467 //
468 // Until 2026-08-27 NOTHING enforced these: repo-bootstrap wrote
469 // requirePullRequest/allowForcePush/allowDeletion on every new repo,
470 // gates.tsx rendered them as configured — and `git push --force main`
471 // sailed through, because evaluateProtection() only ever ran at PR-merge
472 // time. The flow audit called it the platform's headline silent trust
473 // failure. Deletion and require-PR are enforced here (pure ref-name
474 // decisions); force-push detection needs the pushed objects, which are
475 // still quarantined at this point, so it lives in the pre-receive hook
476 // (see buildPreReceiveScript / noForcePushBranches).
477 for (const ref of refs) {
478 if (!ref.refName.startsWith("refs/heads/")) continue;
479 const branch = ref.refName.slice("refs/heads/".length);
480 let rule: Awaited<ReturnType<typeof matchProtection>> = null;
481 try {
482 rule = await matchProtection(repositoryId, branch);
483 } catch {
484 rule = null;
485 }
486 if (!rule) continue;
487
488 if (ref.newSha === ZERO_SHA) {
489 if (!rule.allowDeletion) {
490 violations.push(
491 `branch "${branch}" is protected (pattern: ${rule.pattern}); deletion is not allowed`
492 );
493 }
494 continue; // a deletion is never also a direct-push update
495 }
496
497 // Creation (oldSha zero) is always allowed — blocking it would reject
498 // the very first `git push -u origin main` on a fresh repo.
499 if (rule.requirePullRequest && ref.oldSha !== ZERO_SHA) {
500 violations.push(
501 `branch "${branch}" is protected (pattern: ${rule.pattern}); direct pushes are not allowed — open a pull request and merge it instead`
502 );
503 }
504 }
505
416506 // Rulesets — single DB call, evaluator runs purely on names.
417507 let rulesets: Awaited<ReturnType<typeof listRulesetsForRepo>> = [];
418508 try {
461551 * are no pack-content rules to enforce. Never throws.
462552 */
463553export async function installPackInspectionHookForRepo(
464 repositoryId: string
554 repositoryId: string,
555 refs: RefUpdate[] = []
465556): Promise<{ env: Record<string, string>; cleanup: () => Promise<void> } | null> {
466557 let rulesets: Awaited<ReturnType<typeof listRulesetsForRepo>> = [];
467558 try {
469560 } catch {
470561 rulesets = [];
471562 }
563
564 // Resolve which of the branches being UPDATED (not created, not deleted)
565 // sit under a protection rule with allowForcePush=false. Resolution
566 // happens here in TypeScript via matchProtection — exact-beats-glob and
567 // all — so the bash hook only ever does exact string comparison and the
568 // two languages cannot disagree about glob semantics. Fail-open per
569 // branch: a DB hiccup must not wedge a legitimate push.
570 const noForcePushBranches: string[] = [];
571 for (const ref of refs) {
572 if (!ref.refName.startsWith("refs/heads/")) continue;
573 if (ref.oldSha === ZERO_SHA || ref.newSha === ZERO_SHA) continue;
574 const branch = ref.refName.slice("refs/heads/".length);
575 try {
576 const rule = await matchProtection(repositoryId, branch);
577 if (rule && !rule.allowForcePush) noForcePushBranches.push(branch);
578 } catch {
579 // fail-open
580 }
581 }
582
472583 // Always install — even with zero rulesets — so the unconditional secret
473584 // scan still runs. Only a listRulesetsForRepo throw plus secret-scan being
474585 // disabled entirely skips the hook (handled by installPackInspectionHook's
475 // own early-return when both inputs are empty).
586 // own early-return when every input is empty).
476587 return installPackInspectionHook(rulesets, {
477588 secretScan: !config.secretScanOnPushDisabled,
589 noForcePushBranches,
478590 });
479591}
480592
Modifiedsrc/lib/repo-bootstrap.ts+4−1View fileUnifiedSplit
103103 await db.insert(branchProtection).values({
104104 repositoryId: opts.repositoryId,
105105 pattern: branch,
106 requirePullRequest: true,
106 // False by default since enforcement became real (2026-08-27,
107 // push-policy.ts): true here would reject every solo developer's
108 // first direct `git push origin main`. Opt-in on the gates page.
109 requirePullRequest: false,
107110 requireGreenGates: true,
108111 requireAiApproval: true,
109112 requireHumanReview: false,
Modifiedsrc/lib/ssh-server.ts+7−0View fileUnifiedSplit
469469 let hookEnv: Record<string, string> | undefined;
470470 let hookCleanup: (() => Promise<void>) | undefined;
471471 try {
472 // KNOWN GAP (2026-08-27): no refs are passed, so the branch-protection
473 // enforcement the HTTP path gets (force-push denial via the hook,
474 // deletion/require-PR via evaluatePushPolicy) does NOT run over SSH —
475 // the ref list is only knowable here after the pack streams. SSH is
476 // disabled in production (SSH_PORT=0); before re-enabling it, move
477 // protection matching into the hook itself (pattern file + bash case)
478 // or parse the ref announcements from the channel stream.
472479 const hook = await installPackInspectionHookForRepo(repoInfo.id);
473480 if (hook) {
474481 hookEnv = hook.env;
Modifiedsrc/routes/git.ts+1−1View fileUnifiedSplit
275275 let hookCleanup: (() => Promise<void>) | undefined;
276276 if (refs.length > 0 && cachedRepoId) {
277277 try {
278 const hook = await installPackInspectionHookForRepo(cachedRepoId);
278 const hook = await installPackInspectionHookForRepo(cachedRepoId, refs);
279279 if (hook) {
280280 hookEnv = hook.env;
281281 hookCleanup = hook.cleanup;
282282
c comment · e edit title · m merge · a approve · r request changes · ? shortcuts