diff --git a/extensions/processes/tools/notify.test.ts b/extensions/processes/tools/notify.test.ts index 3983f99..34aa7b2 100644 --- a/extensions/processes/tools/notify.test.ts +++ b/extensions/processes/tools/notify.test.ts @@ -93,6 +93,22 @@ describe("normalizeNotifyConfig", () => { ).toThrow(/pattern must be at most 500 characters/); }); + it("rejects empty log-match patterns", () => { + expect(() => + normalizeNotifyConfig({ + logMatches: [{ pattern: "" }], + }), + ).toThrow(/pattern must not be empty or whitespace-only/); + }); + + it("rejects whitespace-only log-match patterns", () => { + expect(() => + normalizeNotifyConfig({ + logMatches: [{ pattern: " \t " }], + }), + ).toThrow(/pattern must not be empty or whitespace-only/); + }); + it.each([ ["onSuccess", { onSuccess: "bad" }], ["onFailure", { onFailure: "bad" }], diff --git a/extensions/processes/tools/notify.ts b/extensions/processes/tools/notify.ts index 9f19603..1491bdf 100644 --- a/extensions/processes/tools/notify.ts +++ b/extensions/processes/tools/notify.ts @@ -111,6 +111,15 @@ export function normalizeLogMatch( throw new Error(`${actionLabel} ${path}.pattern must be a string`); } + // An empty or whitespace-only literal pattern matches every line + // (String#includes("")), and an empty regex matches every line too. Reject + // early so a stray "" from the model does not fire a notification per line. + if (input.pattern.trim().length === 0) { + throw new Error( + `${actionLabel} ${path}.pattern must not be empty or whitespace-only`, + ); + } + if (input.pattern.length > MAX_NOTIFY_PATTERN_LENGTH) { throw new Error( `${actionLabel} ${path}.pattern must be at most ${MAX_NOTIFY_PATTERN_LENGTH} characters`, diff --git a/src/utils/match-line.test.ts b/src/utils/match-line.test.ts index 9acf35a..3f50162 100644 --- a/src/utils/match-line.test.ts +++ b/src/utils/match-line.test.ts @@ -16,10 +16,13 @@ describe("compileLineMatcher", () => { expect(matcher("an error occurred")).toBe(false); }); - it("matches empty pattern against any line", () => { + it("never matches an empty literal pattern (no match-all footgun)", () => { + // An empty pattern would otherwise match every line via + // String#includes(""), firing a notification per line. Defend at the + // shared primitive; callers should treat "" as "no filter". const matcher = compileLineMatcher("", "literal"); - expect(matcher("anything")).toBe(true); - expect(matcher("")).toBe(true); + expect(matcher("anything")).toBe(false); + expect(matcher("")).toBe(false); }); }); @@ -34,6 +37,15 @@ describe("compileLineMatcher", () => { expect(() => compileLineMatcher("([", "regex")).toThrow(); }); + it("never matches an empty regex pattern (no match-all footgun)", () => { + // An empty regex otherwise matches at every position and would fire a + // notification per line. Defend at the shared primitive; callers should + // treat "" as "no filter". + const matcher = compileLineMatcher("", "regex"); + expect(matcher("anything")).toBe(false); + expect(matcher("")).toBe(false); + }); + it("supports regex flags via pattern", () => { const matcher = compileLineMatcher("error(?=:)", "regex"); expect(matcher("error: something broke")).toBe(true); diff --git a/src/utils/match-line.ts b/src/utils/match-line.ts index 5ab18cc..eb89b9d 100644 --- a/src/utils/match-line.ts +++ b/src/utils/match-line.ts @@ -15,6 +15,14 @@ export function compileLineMatcher( pattern: string, mode: LineMatchMode, ): (line: string) => boolean { + // Guard against the match-all footgun for an empty pattern. An empty + // literal would match every line via String#includes(""), and an empty + // regex matches at every position. Callers should treat "" as "no filter", + // but defend here too so the shared primitive can never become a match-all. + if (pattern.length === 0) { + return () => false; + } + if (mode === "literal") { return (line) => line.includes(pattern); }