fix(push): the per-repo secret-scan toggle now governs pushes, as its label claims #5621
2 changed files+81−1
Addedsrc/__tests__/push-scan-repo-setting.test.ts+58−0View fileUnifiedSplit
@@ -0,0 +1,58 @@
1/**
2 * The per-repo secret-scan toggle governs PUSHES, as its own label claims.
3 *
4 * The gates settings page has always described `secretScanEnabled` as
5 * "Regex + AI secret detection on every push". The merge-time gate honoured
6 * it; the pre-receive hook read only the GLOBAL `secretScanOnPushDisabled`
7 * kill switch and ignored the per-repo setting entirely. A per-repo toggle
8 * that silently governs only half of what it names is the push/merge drift
9 * again — the same family as the `gluecron:allow-secret` pragma that one
10 * gate honoured and the other did not, which armed the 2026-09-01 outage.
11 *
12 * Found the honest way: a sanctioned full-history import (a security
13 * platform's own repository, whose test fixtures necessarily look like
14 * credentials) was rejected at push, and the only off-switch that actually
15 * worked was platform-wide — the wrong blast radius by exactly one platform.
16 *
17 * Structural pins, since the interesting behaviour needs a live DB + a bare
18 * repo + an actual push: what broke was an input never being consulted, so
19 * the tests assert the consultation and its fail direction.
20 */
21import { describe, expect, it } from "bun:test";
22
23const read = () => Bun.file("src/lib/push-policy.ts").text();
24
25describe("installPackInspectionHookForRepo consults the per-repo setting", () => {
26 it("loads getOrCreateSettings and reads secretScanEnabled", async () => {
27 const src = await read();
28 const fn = src.slice(
29 src.indexOf("export async function installPackInspectionHookForRepo"),
30 src.indexOf("export function formatPolicyError")
31 );
32 expect(fn).toContain("getOrCreateSettings");
33 expect(fn).toContain("secretScanEnabled === false");
34 // Both switches must gate the scan: global AND per-repo.
35 expect(fn).toContain("!config.secretScanOnPushDisabled && repoSecretScan");
36 });
37
38 it("fails CLOSED: a settings read failure keeps the scan on", async () => {
39 // The force-push resolution in the same function fails open (a DB hiccup
40 // must not wedge a push); the secret scan must not inherit that trade.
41 // A DB hiccup silently disabling secret scanning is an outage that looks
42 // like nothing happened.
43 const src = await read();
44 const fn = src.slice(
45 src.indexOf("let repoSecretScan = true;"),
46 src.indexOf("// Always install")
47 );
48 expect(fn).toContain("catch");
49 expect(fn).toContain("repoSecretScan = true;");
50 // The default before any lookup is also scanning.
51 expect(fn.startsWith("let repoSecretScan = true;")).toBe(true);
52 });
53
54 it("the merge gate reads the SAME field, so the toggle cannot fork meaning", async () => {
55 const gate = await Bun.file("src/lib/gate.ts").text();
56 expect(gate).toContain("settings?.secretScanEnabled !== false");
57 });
58});
Modifiedsrc/lib/push-policy.ts+23−1View fileUnifiedSplit
@@ -577,12 +577,34 @@ export async function installPackInspectionHookForRepo(
577577 }
578578 }
579579
580 // The per-repo toggle the settings page has always advertised. The gates
581 // UI describes secretScanEnabled as "secret detection on every push", and
582 // the merge-time gate honoured it — but this hook read only the GLOBAL
583 // kill switch, so the per-repo toggle was a lie about pushes. Found when a
584 // sanctioned full-history import (a security platform's own repo, whose
585 // test fixtures necessarily look like credentials) had no per-repo route
586 // past the scan: the only off-switch was platform-wide, which is the wrong
587 // blast radius by exactly one platform.
588 //
589 // Fail direction is CLOSED, per-repo: on any settings read failure the
590 // scan runs. The force-push resolution above fails open because a DB
591 // hiccup must not wedge a legitimate push; a DB hiccup silently disabling
592 // the secret gate is a different trade entirely, and the wrong one.
593 let repoSecretScan = true;
594 try {
595 const { getOrCreateSettings } = await import("./repo-bootstrap");
596 const settings = await getOrCreateSettings(repositoryId);
597 if (settings && settings.secretScanEnabled === false) repoSecretScan = false;
598 } catch {
599 repoSecretScan = true;
600 }
601
580602 // Always install — even with zero rulesets — so the unconditional secret
581603 // scan still runs. Only a listRulesetsForRepo throw plus secret-scan being
582604 // disabled entirely skips the hook (handled by installPackInspectionHook's
583605 // own early-return when every input is empty).
584606 return installPackInspectionHook(rulesets, {
585 secretScan: !config.secretScanOnPushDisabled,
607 secretScan: !config.secretScanOnPushDisabled && repoSecretScan,
586608 noForcePushBranches,
587609 });
588610}
589611
c comment · e edit title · m merge · a approve · r request changes · ? shortcuts