Skip to content

Commit 14f73e8

Browse files
RealDiligentRealDiligent
andauthored
fix(review): accept canary/ptb Discord hosts in loop-escalation webhook validation (#9390)
loop-escalation-wire's ALLOWED_DISCORD_HOSTS listed only discord.com and discordapp.com, while its two sibling validators (alerts.ts, notify-discord.ts) accept the wider, correct four-host set. A valid canary.discord.com or ptb.discord.com DISCORD_WEBHOOK_URL was silently dropped as invalid_global_webhook. Align the constant with the siblings; the host allowlist previously had zero test coverage, now covered directly. Co-authored-by: RealDiligent <nft.gold.eth@gmail.com>
1 parent 995cd78 commit 14f73e8

2 files changed

Lines changed: 23 additions & 2 deletions

File tree

src/review/loop-escalation-wire.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ 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+
// Byte-faithful with src/review/alerts.ts and src/services/notify-discord.ts (#9288): the two extra
32+
// subdomains (canary/ptb) are valid Discord webhook hosts the sibling validators already accept.
33+
const ALLOWED_DISCORD_HOSTS = new Set(["discord.com", "discordapp.com", "canary.discord.com", "ptb.discord.com"]);
3234
const DEFAULT_COOLDOWN_MINUTES = 60;
3335
const AUDIT_EVENT_TYPE = "loop_escalation_notification.discord";
3436
const AUDIT_TARGET_KEY = "fleet:loop-escalation";
@@ -88,7 +90,7 @@ function envString(env: Env, name: string): string | undefined {
8890
return typeof fromEnv === "string" && fromEnv.trim().length > 0 ? fromEnv.trim() : undefined;
8991
}
9092

91-
function isValidDiscordWebhook(url: string): boolean {
93+
export function isValidDiscordWebhook(url: string): boolean {
9294
try {
9395
const parsed = new URL(url);
9496
if (parsed.protocol !== "https:") return false;

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as repositories from "../../src/db/repositories";
33
import {
44
clearLoopEscalationManifestOverrideCacheForTest,
55
isLoopEscalationSweepEnabled,
6+
isValidDiscordWebhook,
67
loadActiveLoopsFromEnv,
78
parseActiveLoopFacts,
89
resolveLoopEscalationManifestOverride,
@@ -13,6 +14,24 @@ import { createTestEnv } from "../helpers/d1";
1314

1415
const SELF_REPO = "JSONbored/loopover";
1516

17+
describe("isValidDiscordWebhook host allowlist (#9288)", () => {
18+
it("accepts all four Discord webhook hosts its sibling validators accept — including canary/ptb", () => {
19+
for (const host of ["discord.com", "discordapp.com", "canary.discord.com", "ptb.discord.com"]) {
20+
// canary/ptb were rejected before the fix, dropping a valid webhook as invalid_global_webhook.
21+
expect(isValidDiscordWebhook(`https://${host}/api/webhooks/123/abc`)).toBe(true);
22+
expect(isValidDiscordWebhook(`https://${host.toUpperCase()}/api/webhooks/123/abc`)).toBe(true); // host match is case-insensitive
23+
}
24+
});
25+
26+
it("still rejects non-Discord hosts, non-https, and non-webhook paths", () => {
27+
expect(isValidDiscordWebhook("https://evil.example.com/api/webhooks/123/abc")).toBe(false);
28+
expect(isValidDiscordWebhook("https://notdiscord.com/api/webhooks/123/abc")).toBe(false);
29+
expect(isValidDiscordWebhook("http://discord.com/api/webhooks/123/abc")).toBe(false); // not https
30+
expect(isValidDiscordWebhook("https://discord.com/not/a/webhook")).toBe(false); // wrong path
31+
expect(isValidDiscordWebhook("not a url")).toBe(false);
32+
});
33+
});
34+
1635
describe("isLoopEscalationSweepEnabled (#6349)", () => {
1736
it("defaults OFF and accepts the standard truthy env forms", () => {
1837
for (const off of [undefined, "", "false", "no", "0", "off"]) {

0 commit comments

Comments
 (0)