From 72230bf38da18426c2f3e4d6e2246490c6537ec4 Mon Sep 17 00:00:00 2001
From: Mike Mooring <mike@divinci.ai>
Date: Thu, 28 May 2026 23:47:45 -0700
Subject: [PATCH] harden(api): escape HTML in email alerts + tighten
 events-endpoint authz
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes the two flagged-for-later items.

1. HTML escaping (alerting.ts): the email channel renders details into an HTML
   body, and some fields are user-controlled — `accountName` traces to
   req.body.name at cloud-account creation, and the cloud-monitoring path's
   `summary` embeds it. So the unescaped interpolation WAS reachable (via the
   core cloud path, not agent-guard). Adds escapeHtml() applied to every string
   interpolation in the email HTML (summary, provider, accountName, violation
   serviceName/metricName/values, actionsTaken). Audited the other channels:
   Slack/Discord/PagerDuty/webhook all transmit via JSON (values render as
   text), so only the email sink needed it. +1 regression test asserting a
   <script> payload is neutralized to &lt;script&gt;.

2. Events authz (agent-guard route): POST /agent-guard/events now requires
   kill_switch:trigger (owner/admin/member) instead of cloud_accounts:read —
   an agent block IS a kill-switch action, so reporting one is a write that
   viewers shouldn't make. GET stays cloud_accounts:read (it's a read).

Full API suite: 845 passed / 12 skipped.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 packages/api/src/routes/agent-guard/index.ts |  5 ++-
 packages/api/src/services/alerting.ts        | 38 +++++++++++++++-----
 packages/api/tests/services/alerting.test.ts | 17 +++++++++
 3 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/packages/api/src/routes/agent-guard/index.ts b/packages/api/src/routes/agent-guard/index.ts
index 3860532..cc870f5 100644
--- a/packages/api/src/routes/agent-guard/index.ts
+++ b/packages/api/src/routes/agent-guard/index.ts
@@ -47,8 +47,11 @@ function clampStr(v: unknown, max = MAX_STR): string | undefined {
 
 /**
  * POST /agent-guard/events — record a budget trip; fan out alerts on block.
+ *
+ * Requires kill_switch:trigger (owner/admin/member): an agent block IS a
+ * kill-switch action, so reporting one is a write that viewers shouldn't make.
  */
-agentGuardRouter.post("/events", requirePermission("cloud_accounts:read"), async (req, res, next) => {
+agentGuardRouter.post("/events", requirePermission("kill_switch:trigger"), async (req, res, next) => {
   try {
     const guardianAccountId = (req as any).guardianAccountId as string;
     const orgId = guardianAccountId;
diff --git a/packages/api/src/services/alerting.ts b/packages/api/src/services/alerting.ts
index ced1995..03c0eaf 100644
--- a/packages/api/src/services/alerting.ts
+++ b/packages/api/src/services/alerting.ts
@@ -12,6 +12,26 @@ type Severity = "critical" | "error" | "warning" | "info";
 
 const GITHUB_API_HOST = "api.github.com";
 
+/**
+ * Escape a value for safe interpolation into HTML. Alert details can carry
+ * user-controlled strings (e.g. a cloud account's `name` from request body,
+ * or a violation's serviceName), and the email channel renders them into an
+ * HTML body — so every interpolated string must be escaped to prevent HTML
+ * injection. Non-string values are stringified first.
+ *
+ * Other channels (Slack/Discord/PagerDuty/webhook) transmit via JSON and render
+ * values as text, so they don't need this; Slack/Discord markdown formatting
+ * from a crafted string is cosmetic only.
+ */
+function escapeHtml(value: unknown): string {
+  return String(value ?? "")
+    .replace(/&/g, "&amp;")
+    .replace(/</g, "&lt;")
+    .replace(/>/g, "&gt;")
+    .replace(/"/g, "&quot;")
+    .replace(/'/g, "&#39;");
+}
+
 /**
  * SSRF Protection: Validate webhook URLs are safe to call.
  * Blocks private/internal IPs and non-HTTPS URLs.
@@ -186,17 +206,17 @@ async function alertEmail(channel: AlertChannel, summary: string, severity: Seve
   const violationRows = violations.map((v: any) => {
     const multiplier = v.threshold > 0 ? `${Math.round(v.currentValue / v.threshold)}×` : "";
     return `<tr>
-      <td style="padding:6px 12px;border-bottom:1px solid #2a2f4a;">${v.serviceName ?? ""}</td>
-      <td style="padding:6px 12px;border-bottom:1px solid #2a2f4a;">${v.metricName ?? ""}</td>
-      <td style="padding:6px 12px;border-bottom:1px solid #2a2f4a;color:#ff6b6b;">${v.currentValue ?? ""}</td>
-      <td style="padding:6px 12px;border-bottom:1px solid #2a2f4a;">${v.threshold ?? ""}</td>
+      <td style="padding:6px 12px;border-bottom:1px solid #2a2f4a;">${escapeHtml(v.serviceName)}</td>
+      <td style="padding:6px 12px;border-bottom:1px solid #2a2f4a;">${escapeHtml(v.metricName)}</td>
+      <td style="padding:6px 12px;border-bottom:1px solid #2a2f4a;color:#ff6b6b;">${escapeHtml(v.currentValue)}</td>
+      <td style="padding:6px 12px;border-bottom:1px solid #2a2f4a;">${escapeHtml(v.threshold)}</td>
       <td style="padding:6px 12px;border-bottom:1px solid #2a2f4a;font-weight:bold;color:#ff6b6b;">${multiplier}</td>
     </tr>`;
   }).join("");
 
   const actionsTaken = (details.actionsTaken as string[] | undefined) ?? [];
   const actionsHtml = actionsTaken.length > 0
-    ? `<p style="color:#4ade80;"><strong>Kill Switch action taken:</strong> ${actionsTaken.join(", ")}</p>`
+    ? `<p style="color:#4ade80;"><strong>Kill Switch action taken:</strong> ${actionsTaken.map(escapeHtml).join(", ")}</p>`
     : "";
 
   const html = `<!DOCTYPE html>
@@ -212,11 +232,11 @@ async function alertEmail(channel: AlertChannel, summary: string, severity: Seve
     </p>
 
     <div style="background:rgba(255,107,107,0.08);border:1px solid rgba(255,107,107,0.2);border-radius:8px;padding:16px;margin-bottom:24px;">
-      <p style="color:#fff;font-size:15px;font-weight:600;margin:0 0 4px;">${summary}</p>
+      <p style="color:#fff;font-size:15px;font-weight:600;margin:0 0 4px;">${escapeHtml(summary)}</p>
       <p style="color:#9ca3af;font-size:13px;margin:0;">
-        Provider: <strong style="color:#fff;">${String(details.provider ?? "")}</strong> ·
-        Account: <strong style="color:#fff;">${String(details.accountName ?? "")}</strong> ·
-        Severity: <strong style="color:#ff6b6b;">${severity}</strong>
+        Provider: <strong style="color:#fff;">${escapeHtml(details.provider)}</strong> ·
+        Account: <strong style="color:#fff;">${escapeHtml(details.accountName)}</strong> ·
+        Severity: <strong style="color:#ff6b6b;">${escapeHtml(severity)}</strong>
       </p>
     </div>
 
diff --git a/packages/api/tests/services/alerting.test.ts b/packages/api/tests/services/alerting.test.ts
index 6f8977c..7ade0c6 100644
--- a/packages/api/tests/services/alerting.test.ts
+++ b/packages/api/tests/services/alerting.test.ts
@@ -440,6 +440,23 @@ describe("Alerting Service", () => {
       expect(payload.text).toBeDefined();
     });
 
+    it("escapes HTML in user-controlled fields (accountName, serviceName, summary)", async () => {
+      // accountName traces to req.body.name at cloud-account creation — user-controlled.
+      const xss = `<script>alert('x')</script>`;
+      await sendAlerts([emailChannel()], `summary ${xss}`, "critical", {
+        provider: "cloudflare",
+        accountName: `acct ${xss}`,
+        violations: [{ serviceName: `svc ${xss}`, metricName: "CPU", currentValue: 1, threshold: 0, severity: "critical" }],
+        actionsTaken: [`disconnect ${xss}`],
+      });
+
+      const [payload] = mockResendSend.mock.calls[0];
+      // The raw <script> tag must never appear in the rendered HTML…
+      expect(payload.html).not.toContain("<script>");
+      // …but the escaped form should, proving the content is preserved, just neutralized.
+      expect(payload.html).toContain("&lt;script&gt;");
+    });
+
     it("subject includes severity emoji and uppercased severity", async () => {
       await sendAlerts([emailChannel()], "Cost runaway", "critical", {
         provider: "cloudflare", accountName: "zombay-cf", violations,