Skip to content

Commit d48341f

Browse files
committed
fix(review): widen loop-escalation-wire's Discord host allowlist to match its siblings
ALLOWED_DISCORD_HOSTS only had discord.com/discordapp.com, unlike the identical four-host set (adding canary.discord.com/ptb.discord.com) already used by alerts.ts and notify-discord.ts, so a valid canary or PTB webhook was silently rejected as invalid_global_webhook with no indication why.
1 parent aad3f93 commit d48341f

2 files changed

Lines changed: 31 additions & 1 deletion

File tree

src/review/loop-escalation-wire.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import { loadRepoFocusManifest } from "../signals/focus-manifest-loader";
2828
import { resolveLoopOverSelfRepoFullName } from "../config/loopover-repo-focus-manifest";
2929
import { errorMessage } from "../utils/json";
3030

31-
const ALLOWED_DISCORD_HOSTS = new Set(["discord.com", "discordapp.com"]);
31+
const ALLOWED_DISCORD_HOSTS = new Set(["discord.com", "discordapp.com", "canary.discord.com", "ptb.discord.com"]);
3232
const DEFAULT_COOLDOWN_MINUTES = 60;
3333
const AUDIT_EVENT_TYPE = "loop_escalation_notification.discord";
3434
const AUDIT_TARGET_KEY = "fleet:loop-escalation";

test/unit/loop-escalation-wire.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,36 @@ describe("runLoopEscalationSweep (#6349)", () => {
309309
).resolves.toMatchObject({ notified: false, reason: "invalid_global_webhook" });
310310
});
311311

312+
// #9288: ALLOWED_DISCORD_HOSTS previously only had discord.com/discordapp.com, unlike alerts.ts and
313+
// notify-discord.ts's four-host set -- a canary.discord.com or ptb.discord.com webhook was silently
314+
// rejected as invalid_global_webhook. Both must now be accepted the same way the other two paths accept
315+
// them, and a non-Discord host must still be rejected (the "unknown hosts" regression guard above already
316+
// covers that, but this test pins it alongside the newly-accepted hosts for a single clear diff anchor).
317+
it("accepts canary.discord.com and ptb.discord.com webhooks, and still rejects a non-Discord host (#9288)", async () => {
318+
vi.spyOn(console, "error").mockImplementation(() => undefined);
319+
const load = () => [{ loopId: "broken", tenantId: "acme", runStatus: "abandoned" as const }];
320+
321+
await expect(
322+
runLoopEscalationSweep(createTestEnv({ DISCORD_WEBHOOK_URL: "https://canary.discord.com/api/webhooks/123/abc" }), {
323+
loadActiveLoops: load,
324+
fetchImpl: (async () => new Response(null, { status: 204 })) as typeof fetch,
325+
}),
326+
).resolves.toMatchObject({ notified: true });
327+
328+
await expect(
329+
runLoopEscalationSweep(createTestEnv({ DISCORD_WEBHOOK_URL: "https://ptb.discord.com/api/webhooks/123/abc" }), {
330+
loadActiveLoops: load,
331+
fetchImpl: (async () => new Response(null, { status: 204 })) as typeof fetch,
332+
}),
333+
).resolves.toMatchObject({ notified: true });
334+
335+
await expect(
336+
runLoopEscalationSweep(createTestEnv({ DISCORD_WEBHOOK_URL: "https://fake.discord.com/api/webhooks/123/abc" }), {
337+
loadActiveLoops: load,
338+
}),
339+
).resolves.toMatchObject({ notified: false, reason: "invalid_global_webhook" });
340+
});
341+
312342
it("honors an explicit cooldownMinutes override", async () => {
313343
vi.spyOn(console, "error").mockImplementation(() => undefined);
314344
const env = createTestEnv({ DISCORD_WEBHOOK_URL: "https://discord.com/api/webhooks/123/abc" });

0 commit comments

Comments
 (0)