fix(review): treat an empty consent phrase as unconfigured, not auto-satisfied

src/review/cla-check.ts

@@ -47,11 +47,15 @@
  * finding into the advisory before the gate evaluates.
  */
 export function evaluateClaCheck(
   config: ClaCheckConfig,
   ctx: { body?: string | null | undefined; checkRunConclusion?: string | null | undefined },
 ): AdvisoryFinding[] {
-  if (config.consentPhrase === null && config.checkRunName === null) return []; // nothing configured ⇒ no finding
-  const phraseSatisfied = config.consentPhrase !== null && (ctx.body ?? "").toLowerCase().includes(config.consentPhrase.toLowerCase());
+  // An empty-string consentPhrase is not a usable detection method: a case-insensitive `.includes("")` matches
+  // EVERY body, which would silently satisfy CLA consent for every PR. Treat it as "phrase detection not
+  // configured" (same as null) — so it never satisfies consent, and never emits a nonsensical `must contain ""`.
+  const consentPhrase = config.consentPhrase === "" ? null : config.consentPhrase;
+  if (consentPhrase === null && config.checkRunName === null) return []; // nothing configured ⇒ no finding
+  const phraseSatisfied = consentPhrase !== null && (ctx.body ?? "").toLowerCase().includes(consentPhrase.toLowerCase());
   const checkRunSatisfied = config.checkRunName !== null && (ctx.checkRunConclusion === "success" || ctx.checkRunConclusion === "neutral");
   if (phraseSatisfied || checkRunSatisfied) return [];
   // A configured check-run whose conclusion is unresolved: cannot confirm OR deny consent via that method, so
@@ -69,7 +73,7 @@
     ];
   }
   const missing: string[] = [];
-  if (config.consentPhrase !== null) missing.push(`the PR description must contain "${config.consentPhrase}"`);
+  if (consentPhrase !== null) missing.push(`the PR description must contain "${consentPhrase}"`);
   if (config.checkRunName !== null) missing.push(`the "${config.checkRunName}" check must pass`);
   return [
     {

test/unit/cla-check.test.ts

@@ -110,7 +110,27 @@
         checkRunConclusion: undefined,
       });
       expect(out).toHaveLength(1);
       expect(out[0]?.code).toBe(CLA_CHECK_UNRESOLVED_CODE);
     });
   });
+
+  // Regression: an empty-string consentPhrase (distinct from null) must NOT unconditionally satisfy consent —
+  // `"".includes("")`/any `body.includes("")` is always true, which previously bypassed CLA for every PR. An
+  // empty phrase is treated as "phrase detection not configured" (same as null).
+  describe("empty-string consentPhrase is treated as not configured (regression)", () => {
+    it("does NOT satisfy consent: an empty phrase alongside a failing check-run still fails, and never lists a nonsensical empty phrase", () => {
+      const out = evaluateClaCheck(config({ consentPhrase: "", checkRunName: "CLA Assistant Lite" }), {
+        body: "any body at all",
+        checkRunConclusion: "failure",
+      });
+      expect(out).toHaveLength(1);
+      expect(out[0]?.code).toBe(CLA_CONSENT_MISSING_CODE);
+      expect(out[0]?.detail).toContain('the "CLA Assistant Lite" check must pass');
+      expect(out[0]?.detail).not.toContain('must contain ""');
+    });
+
+    it("with no other method configured, an empty phrase is 'nothing configured' → no finding (not a silent bypass, not a nonsensical failure)", () => {
+      expect(evaluateClaCheck(config({ consentPhrase: "" }), { body: "anything" })).toEqual([]);
+    });
+  });
 });