fix(gate): unconditional secret-leak block; don't let inconclusive AI bury blockers

src/queue/processors.ts

@@ -172,7 +172,7 @@ import { loadRepoFocusManifest } from "../signals/focus-manifest-loader";
 import { resolveRepositorySettings } from "../settings/repository-settings";
 import type { LocalBranchAnalysisInput } from "../signals/local-branch";
 import { runGittensoryAiReview } from "../services/ai-review";
-import { isSafetyEnabled, secretLeakFinding } from "../review/safety";
+import { secretLeakFinding } from "../review/safety";
 import { buildReviewGroundingText, checkSummaryText as checkFailureSummaryText, isGroundingEnabled } from "../review/grounding-wire";
 import { buildReviewRagContext, isRagEnabled } from "../review/rag-wire";
 import { indexRepo, reindexChangedPaths } from "../review/rag-index";
@@ -2030,10 +2030,10 @@ export async function maybeAddSecretLeakFinding(
     files: Awaited<ReturnType<typeof listPullRequestFiles>> | null;
   },
 ): Promise<void> {
-  // Per-repo cutover gate (GITTENSORY_REVIEW_REPOS): the secret-leak scan activates for THIS repo only when
-  // it is allowlisted AND the global safety flag is ON. Empty/unset allowlist → no-op for every repo (the
-  // advisory is byte-identical to today) regardless of GITTENSORY_REVIEW_SAFETY.
-  if (!isSafetyEnabled(env) || !isConvergenceRepoAllowed(env, args.repoFullName)) return;
+  // UNCONDITIONAL (#audit-3.4): a CONCRETE, real-format committed credential (github_token, aws_access_key, …)
+  // is unambiguously a leak regardless of which repo it lands in, so the secret-leak hard block runs for every
+  // repo — NOT only the safety-flag-on / allowlisted ones. secretLeakFinding already filters to HARD_SECRET_KINDS
+  // (the weak heuristics that false-positive on config/workflow content are dropped), so this never mis-fires.
   try {
     const files = args.files ?? (await listPullRequestFiles(env, args.repoFullName, args.pullNumber));
     const finding = secretLeakFinding(buildSecretScanDiff(files));
@@ -2408,17 +2408,14 @@ async function maybePublishPrPublicSurface(
       });
     }
 
-    // Safety secrets-scan (convergence, flag-gated by GITTENSORY_REVIEW_SAFETY). Scans the diff and, on a hit,
-    // appends a critical `secret_leak` blocker BEFORE the gate evaluates. When the scan will actually run
-    // (safety flag ON + repo allowlisted), pass the shared resolved files so it scans the REAL diff (FIX B);
-    // otherwise pass the already-loaded files (or null) and let the scan early-return. Flag-OFF (default) is an
-    // immediate no-op → the advisory/gate is byte-identical.
-    const safetyWillRun = isSafetyEnabled(env) && isConvergenceRepoAllowed(env, repoFullName);
+    // Secrets-scan (#audit-3.4): always scans the REAL resolved diff and, on a CONCRETE credential hit, appends a
+    // critical `secret_leak` hard blocker BEFORE the gate evaluates — unconditionally, since a committed token is
+    // a leak on any repo. getReviewFiles() is memoized, so this reuses the already-loaded diff when present.
     await maybeAddSecretLeakFinding(env, {
       advisory,
       repoFullName,
       pullNumber: pr.number,
-      files: safetyWillRun ? await getReviewFiles() : reviewFiles,
+      files: await getReviewFiles(),
     });
 
     // First-time-contributor grace (#552): compute the author's complete per-repo PR history

src/review/safety.ts

@@ -71,8 +71,9 @@ export function defangReviewInput(input: SafetyReviewInput): {
  * (see rules/advisory.ts) so a leaked secret holds the PR. Only CONCRETE credential formats
  * ({@link HARD_SECRET_KINDS}) qualify — the weak `seed_or_mnemonic` / `bittensor_key` heuristics are ignored
  * here because they false-positive on legitimate config/workflow content (e.g. `coldkey:` / `hotkey =` lines
- * in *.toml, .github/workflows/**, or wrangler/workers config). Callers MUST gate this on
- * {@link isSafetyEnabled} — when OFF, no finding is produced so the advisory/gate is unchanged.
+ * in *.toml, .github/workflows/**, or wrangler/workers config). This is UNCONDITIONAL (#audit-3.4): a concrete,
+ * real-format committed credential is a leak on any repo, so the caller runs it regardless of the safety flag /
+ * review allowlist (unlike the prompt-injection defang, which stays flag-gated).
  */
 export function secretLeakFinding(diff: string): AdvisoryFinding | null {
   // Scan ONLY additions — the secrets THIS change introduces. A token on a removed/context line is not being

src/rules/advisory.ts

@@ -359,19 +359,6 @@ export function evaluateGateCheck(advisoryResult: Advisory, policy: GateCheckPol
       warnings,
     };
   }
-  // Fail-CLOSED AI hold (#ai-fail-closed): the block-mode AI review could not return a usable verdict, so the
-  // gate is HELD (neutral) for a human rather than passed automatically. NEVER a failure → a contributor PR is
-  // never auto-CLOSED because an AI model hiccupped; it re-evaluates on the next update.
-  if (advisoryResult.findings.some((finding) => finding.code === "ai_review_inconclusive")) {
-    return {
-      enabled: true,
-      conclusion: "neutral",
-      title: "Gittensory Gate — held for human review",
-      summary: "The AI review could not be completed for this change, so the gate is held for a human reviewer rather than passed automatically. It re-evaluates on the next update.",
-      blockers: [],
-      warnings,
-    };
-  }
   // Merge-readiness composite (#551): when set, escalate every sub-gate to its mode so they roll into one
   // pass/fail. When off, this is a no-op and each sub-gate keeps its own mode.
   const effective = applyMergeReadinessGate(policy);
@@ -404,6 +391,21 @@ export function evaluateGateCheck(advisoryResult: Advisory, policy: GateCheckPol
     };
   }
   if (blockers.length === 0) {
+    // Fail-CLOSED AI hold (#ai-fail-closed, #audit-3.5): with NO deterministic blocker, a block-mode AI review
+    // that could not return a usable verdict HOLDS the gate (neutral) for a human rather than passing
+    // automatically — NEVER a failure, so a contributor PR is never auto-CLOSED because a model hiccupped. This
+    // is evaluated AFTER the deterministic blockers above, so a real violation (secret_leak, duplicate,
+    // missing-issue, slop, quality) still blocks: an inconclusive AI can no longer bury a blocked PR in a hold.
+    if (advisoryResult.findings.some((finding) => finding.code === "ai_review_inconclusive")) {
+      return {
+        enabled: true,
+        conclusion: "neutral",
+        title: "Gittensory Gate — held for human review",
+        summary: "The AI review could not be completed for this change, so the gate is held for a human reviewer rather than passed automatically. It re-evaluates on the next update.",
+        blockers: [],
+        warnings,
+      };
+    }
     return {
       enabled: true,
       conclusion: "success",

test/unit/gate-check-policy.test.ts

@@ -110,6 +110,20 @@ describe("AI fail-closed hold (#ai-fail-closed)", () => {
     expect(result.conclusion).toBe("neutral");
     expect(result.blockers).toEqual([]);
   });
+
+  it("a deterministic hard blocker (secret_leak) still FAILS even when the AI review is inconclusive (#audit-3.5)", () => {
+    const adv: Advisory = {
+      ...missingIssueAdvisory(),
+      findings: [
+        { code: "secret_leak", title: "Possible leaked secret", severity: "critical", detail: "a committed token", action: "remove and rotate it" },
+        { code: "ai_review_inconclusive", title: "AI review could not be completed", severity: "warning", detail: "no usable verdict", action: "held for human" },
+      ],
+    };
+    const result = evaluateGateCheck(adv, gateCheckPolicy(settings(), null, true));
+    // An inconclusive AI can no longer bury a real violation in a "held" state — the secret_leak still hard-blocks.
+    expect(result.conclusion).toBe("failure");
+    expect(result.blockers.map((blocker) => blocker.code)).toContain("secret_leak");
+  });
 });
 
 describe("policy pack (#692)", () => {

test/unit/safety-wiring.test.ts

@@ -255,7 +255,7 @@ describe("secret-leak finding in the advisory build", () => {
     expect(out).toContain("### ok.ts (added) +2/-1\n@@\n+const a = 1;");
   });
 
-  it("FLAG-OFF (default): no secret_leak finding is produced — the advisory is unchanged", async () => {
+  it("FLAG-OFF: a concrete leaked secret STILL produces the secret_leak finding (unconditional, #audit-3.4)", async () => {
     const env = createTestEnv({ GITTENSORY_REVIEW_SAFETY: "false" });
     const adv = advisory();
     const files = [
@@ -278,7 +278,8 @@ describe("secret-leak finding in the advisory build", () => {
       pullNumber: 7,
       files,
     });
-    expect(adv.findings).toEqual([]);
+    // The concrete-credential hard block does not depend on GITTENSORY_REVIEW_SAFETY.
+    expect(adv.findings.map((f) => f.code)).toContain("secret_leak");
   });
 
   it("FLAG-ON + files=null: lazily loads the changed files from D1 and still finds the leaked secret", async () => {

test/unit/safety.test.ts

@@ -130,7 +130,7 @@ describe("GITTENSORY_REVIEW_SAFETY secrets-scan wired into the review FINALIZE p
     expect(seen.conclusion).toBe("failure");
   });
 
-  it("FLAG-OFF (default): the SAME leaked secret produces NO blocker — the finalized gate is byte-identical (not failed on the secret)", async () => {
+  it("FLAG-OFF: a leaked secret STILL fails the gate — the concrete-credential block is unconditional (#audit-3.4)", async () => {
     const env = createTestEnv({ GITTENSORY_REVIEW_SAFETY: "false", GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() });
     await seedGateEnabledRepo(env);
     await seedLeakedSecretFile(env);
@@ -141,11 +141,12 @@ describe("GITTENSORY_REVIEW_SAFETY secrets-scan wired into the review FINALIZE p
     } finally {
       vi.unstubAllGlobals();
     }
-    // No secret_leak finding is produced flag-OFF → the gate is not driven to failure by the (ignored) secret.
-    expect(seen.conclusion).not.toBe("failure");
+    // A real-format committed credential is a leak on any repo, so the secret_leak hard block fires regardless
+    // of GITTENSORY_REVIEW_SAFETY (only the prompt-injection defang / AI review remain flag-gated).
+    expect(seen.conclusion).toBe("failure");
   });
 
-  it("UNSET behaves identically to explicit-false (the flag-OFF branch is the default — no new branch taken)", async () => {
+  it("UNSET (default): a leaked secret also fails the gate — the secret-leak block does not depend on the flag", async () => {
     const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: await generatePrivateKeyPem() }); // GITTENSORY_REVIEW_SAFETY unset
     await seedGateEnabledRepo(env);
     await seedLeakedSecretFile(env);
@@ -156,6 +157,6 @@ describe("GITTENSORY_REVIEW_SAFETY secrets-scan wired into the review FINALIZE p
     } finally {
       vi.unstubAllGlobals();
     }
-    expect(seen.conclusion).not.toBe("failure");
+    expect(seen.conclusion).toBe("failure");
   });
 });