From 4af9136c98c1667a9ed63c985e510d990fbbfb6a Mon Sep 17 00:00:00 2001 From: Matthew McConnell Date: Mon, 19 Oct 2026 22:50:37 +0100 Subject: [PATCH 1/5] Alerts: defuse mentions and links in the user's email --- apps/api/src/lib/alerts.ts | 11 ++++++++++- apps/api/test/alerts.test.ts | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/apps/api/src/lib/alerts.ts b/apps/api/src/lib/alerts.ts index f92162d..6d363d5 100644 --- a/apps/api/src/lib/alerts.ts +++ b/apps/api/src/lib/alerts.ts @@ -40,6 +40,14 @@ const SEND_TIMEOUT_MS = 30_000; /** Failed port-22 checks in a row before a host counts as offline. */ const DOWN_AFTER = 2; +// One text goes to every service: lookalikes and zero-width spaces stop Slack ``/``, `@channel` and markdown links without showing escapes in plain-text ones. +const defuse = (s: string) => + s + .replaceAll("<", "‹") + .replaceAll(">", "›") + .replace(/(? Object.fromEntries( Object.entries(process.env).filter( @@ -213,7 +221,8 @@ export function alerter({ .get(String(id)); const hostName = (e: AuditInput) => (e.name as string | undefined) ?? host(e.host)?.name ?? String(e.host); - const who = (e: AuditInput) => String(e.email ?? e.sub ?? "someone"); + const who = (e: AuditInput) => + defuse(String(e.email ?? e.sub ?? "someone")); function securityMessage(e: AuditInput): Message { const lines = [ diff --git a/apps/api/test/alerts.test.ts b/apps/api/test/alerts.test.ts index 6156d54..b2fdfb8 100644 --- a/apps/api/test/alerts.test.ts +++ b/apps/api/test/alerts.test.ts @@ -307,6 +307,21 @@ test("terminal event: filtered by host tag or login; empty filter = every termin ]); }); +test("terminal event: mentions and links in the email arrive defused", async () => { + await setup(); + await create({ events: ["terminal"] }); + audit(ctx.db, { + event: "session_start", + sub: "u", + email: " @channel [x](https://evil.test)", + principal: "ws:h1:root", + session: "s1", + }); + expect((await sent(1))[0]?.args[2]).toBe( + "CertShell: ‹!here› @​channel [x]​(https://evil.test) opened root@h1", + ); +}); + test("task_failed: one alert when the last host finishes, only if one failed", async () => { await setup({ command: ({ host, remote }) => ["sh", "-c", remote ?? "", host.id], From 6aaf06174770930e51397729b0f28b3f4c63b645 Mon Sep 17 00:00:00 2001 From: Matthew McConnell Date: Mon, 19 Oct 2026 22:51:01 +0100 Subject: [PATCH 2/5] Alerts: defuse mentions and links in task names and run emails --- apps/api/src/lib/alerts.ts | 4 ++-- apps/api/test/alerts.test.ts | 27 +++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 2 deletions(-) diff --git a/apps/api/src/lib/alerts.ts b/apps/api/src/lib/alerts.ts index 6d363d5..27df995 100644 --- a/apps/api/src/lib/alerts.ts +++ b/apps/api/src/lib/alerts.ts @@ -271,9 +271,9 @@ export function alerter({ >("SELECT name, email, sub FROM runs WHERE id = ?") .get(runId); return { - title: `CertShell: task "${run?.name}" failed on ${failed.length} of ${hosts.length} host${hosts.length === 1 ? "" : "s"}`, + title: `CertShell: task "${defuse(String(run?.name))}" failed on ${failed.length} of ${hosts.length} host${hosts.length === 1 ? "" : "s"}`, body: [ - `Run by ${run?.email ?? run?.sub}.`, + `Run by ${defuse(String(run?.email ?? run?.sub))}.`, ...failed.map( (h) => `${h.login}@${h.host_name}: ${h.exit_code === null ? "didn't finish" : `exit ${h.exit_code}`}`, diff --git a/apps/api/test/alerts.test.ts b/apps/api/test/alerts.test.ts index b2fdfb8..3f564aa 100644 --- a/apps/api/test/alerts.test.ts +++ b/apps/api/test/alerts.test.ts @@ -382,6 +382,33 @@ test("task_failed: one alert when the last host finishes, only if one failed", a ); }); +test("task_failed: mentions and links in the task name arrive defused", async () => { + await setup({ command: () => ["sh", "-c", "exit 1"] }); + await create({ events: ["task_failed"] }); + const headers = { + cookie: ctx.cookie, + origin: APP_URL, + "content-type": "application/json", + }; + const res = await ctx.app.request(`${APP_URL}/api/tasks`, { + method: "POST", + headers, + body: JSON.stringify({ + name: " ", + script: "true", + targets: [{ host: "h1", login: "root" }], + }), + }); + const { id } = (await res.json()) as { id: number }; + await ctx.app.request(`${APP_URL}/api/tasks/${id}/run`, { + method: "POST", + headers, + }); + expect((await sent(1))[0]?.args[2]).toBe( + 'CertShell: task "‹!channel› ‹https://evil.test|x›" failed on 1 of 1 host', + ); +}); + test("reboot: alerted when host facts first show the flag, not again until it clears", async () => { let reboot = "1"; await setup({ command: () => ["sh", "-c", `echo reboot=${reboot}`] }); From efce5fedc89628d11c003b869fa63b83cf0d3a4d Mon Sep 17 00:00:00 2001 From: Matthew McConnell Date: Mon, 19 Oct 2026 22:56:51 +0100 Subject: [PATCH 3/5] Alerts tests: one task POST helper instead of per-test request closures --- apps/api/test/alerts.test.ts | 40 ++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/apps/api/test/alerts.test.ts b/apps/api/test/alerts.test.ts index 3f564aa..2d47c9a 100644 --- a/apps/api/test/alerts.test.ts +++ b/apps/api/test/alerts.test.ts @@ -48,6 +48,19 @@ const call = (method: string, path: string, body?: unknown) => body: body === undefined ? undefined : JSON.stringify(body), }); +const post = async (path: string, body: unknown, cookie = ctx.cookie) => + (await ( + await ctx.app.request(`${APP_URL}/api/tasks${path}`, { + method: "POST", + headers: { + cookie, + origin: APP_URL, + "content-type": "application/json", + }, + body: JSON.stringify(body), + }) + ).json()) as { id: number | string }; + const input = (over: Partial = {}): AlertInput => ({ name: "ops", url: "ntfy://ntfy.sh/secret-topic", @@ -330,32 +343,19 @@ test("task_failed: one alert when the last host finishes, only if one failed", a root: "certshell-admins", }); await create({ events: ["task_failed"] }); - const task = async (script: string) => { - const r = await ctx.app.request(`${APP_URL}/api/tasks`, { - method: "POST", - headers: { - cookie: ctx.cookie, - origin: APP_URL, - "content-type": "application/json", - }, - body: JSON.stringify({ + const task = async (script: string) => + ( + await post("", { name: "Upgrade", script, targets: [ { host: "h1", login: "root" }, { host: "h2", login: "root" }, ], - }), - }); - return ((await r.json()) as { id: number }).id; - }; - const run = async (id: number) => { - const r = await ctx.app.request(`${APP_URL}/api/tasks/${id}/run`, { - method: "POST", - headers: { cookie: ctx.cookie, origin: APP_URL }, - }); - return ((await r.json()) as { id: string }).id; - }; + }) + ).id; + const run = async (id: number | string) => + String((await post(`/${id}/run`, {})).id); const done = async (runId: string) => { for (let i = 0; i < 100; i++) { From 47057b35313fac7ea486876246d7c777e2339fb7 Mon Sep 17 00:00:00 2001 From: Matthew McConnell Date: Mon, 19 Oct 2026 22:56:56 +0100 Subject: [PATCH 4/5] Alerts: defuse whole messages, _@ mentions, control chars and email newlines --- apps/api/src/lib/alerts.ts | 13 ++++++----- apps/api/test/alerts.test.ts | 43 +++++++++++++++++++----------------- 2 files changed, 31 insertions(+), 25 deletions(-) diff --git a/apps/api/src/lib/alerts.ts b/apps/api/src/lib/alerts.ts index 27df995..d7bafec 100644 --- a/apps/api/src/lib/alerts.ts +++ b/apps/api/src/lib/alerts.ts @@ -45,8 +45,9 @@ const defuse = (s: string) => s .replaceAll("<", "‹") .replaceAll(">", "›") - .replace(/(? Object.fromEntries( @@ -221,8 +222,9 @@ export function alerter({ .get(String(id)); const hostName = (e: AuditInput) => (e.name as string | undefined) ?? host(e.host)?.name ?? String(e.host); + // An IdP-editable email with a newline could forge alert lines. const who = (e: AuditInput) => - defuse(String(e.email ?? e.sub ?? "someone")); + String(e.email ?? e.sub ?? "someone").replaceAll("\n", " "); function securityMessage(e: AuditInput): Message { const lines = [ @@ -271,9 +273,9 @@ export function alerter({ >("SELECT name, email, sub FROM runs WHERE id = ?") .get(runId); return { - title: `CertShell: task "${defuse(String(run?.name))}" failed on ${failed.length} of ${hosts.length} host${hosts.length === 1 ? "" : "s"}`, + title: `CertShell: task "${run?.name}" failed on ${failed.length} of ${hosts.length} host${hosts.length === 1 ? "" : "s"}`, body: [ - `Run by ${defuse(String(run?.email ?? run?.sub))}.`, + `Run by ${who({ event: "", email: run?.email, sub: run?.sub })}.`, ...failed.map( (h) => `${h.login}@${h.host_name}: ${h.exit_code === null ? "didn't finish" : `exit ${h.exit_code}`}`, @@ -323,6 +325,7 @@ export function alerter({ }; } else return; if (!msg) return; + msg = { title: defuse(msg.title), body: defuse(msg.body) }; for (const row of rows()) { const c = channelOf(row); if (c.events.includes(event) && matches(c)) void send(row, msg); diff --git a/apps/api/test/alerts.test.ts b/apps/api/test/alerts.test.ts index 2d47c9a..4318991 100644 --- a/apps/api/test/alerts.test.ts +++ b/apps/api/test/alerts.test.ts @@ -4,6 +4,8 @@ import { tmpdir } from "node:os"; import type { AlertChannel, AlertInput } from "@repo/shared"; import { alerter } from "../src/lib/alerts"; import { audit } from "../src/lib/audit"; +import { createSession } from "../src/lib/sessions"; +import { SESSION_COOKIE } from "../src/routes/auth"; import { hostKeyRejected } from "../src/ssh/terminal"; import { APP_URL, startApp } from "./helpers/app"; @@ -326,12 +328,12 @@ test("terminal event: mentions and links in the email arrive defused", async () audit(ctx.db, { event: "session_start", sub: "u", - email: " @channel [x](https://evil.test)", + email: " _@channel [x](https://evil.test)\nfake\u0007line", principal: "ws:h1:root", session: "s1", }); expect((await sent(1))[0]?.args[2]).toBe( - "CertShell: ‹!here› @​channel [x]​(https://evil.test) opened root@h1", + "CertShell: ‹!here› _@\u200bchannel [x]\u200b(https://evil.test) fake line opened root@h1", ); }); @@ -382,31 +384,32 @@ test("task_failed: one alert when the last host finishes, only if one failed", a ); }); -test("task_failed: mentions and links in the task name arrive defused", async () => { +test("task_failed: mentions and links in the task name and email arrive defused", async () => { await setup({ command: () => ["sh", "-c", "exit 1"] }); await create({ events: ["task_failed"] }); - const headers = { - cookie: ctx.cookie, - origin: APP_URL, - "content-type": "application/json", - }; - const res = await ctx.app.request(`${APP_URL}/api/tasks`, { - method: "POST", - headers, - body: JSON.stringify({ + const cookie = `${SESSION_COOKIE}=${createSession(ctx.db, { + iss: "https://id.test", + sub: "user-2", + email: "\n_@here", + groups: ["certshell-admins"], + })}`; + const { id } = await post( + "", + { name: " ", script: "true", targets: [{ host: "h1", login: "root" }], - }), - }); - const { id } = (await res.json()) as { id: number }; - await ctx.app.request(`${APP_URL}/api/tasks/${id}/run`, { - method: "POST", - headers, - }); - expect((await sent(1))[0]?.args[2]).toBe( + }, + cookie, + ); + const runId = (await post(`/${id}/run`, {}, cookie)).id; + const [s] = await sent(1); + expect(s?.args[2]).toBe( 'CertShell: task "‹!channel› ‹https://evil.test|x›" failed on 1 of 1 host', ); + expect(s?.body).toBe( + `Run by ‹!channel› _@\u200bhere.\nroot@h1: exit 1\nhttps://ssh.test/tasks/runs/${runId}`, + ); }); test("reboot: alerted when host facts first show the flag, not again until it clears", async () => { From 4befe676d91c7d03cba9c9c509961ab03f695c82 Mon Sep 17 00:00:00 2001 From: Matthew McConnell Date: Mon, 19 Oct 2026 22:58:51 +0100 Subject: [PATCH 5/5] Alerts: break URLs and line separators in user-set text --- apps/api/src/lib/alerts.ts | 13 ++++++++----- apps/api/test/alerts.test.ts | 10 +++++----- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/apps/api/src/lib/alerts.ts b/apps/api/src/lib/alerts.ts index d7bafec..93dab01 100644 --- a/apps/api/src/lib/alerts.ts +++ b/apps/api/src/lib/alerts.ts @@ -222,9 +222,12 @@ export function alerter({ .get(String(id)); const hostName = (e: AuditInput) => (e.name as string | undefined) ?? host(e.host)?.name ?? String(e.host); - // An IdP-editable email with a newline could forge alert lines. - const who = (e: AuditInput) => - String(e.email ?? e.sub ?? "someone").replaceAll("\n", " "); + // User-set text (task name, IdP-editable email): a newline could forge alert lines, a URL would autolink. + const userText = (s: unknown) => + String(s) + .replace(/[\n\u2028\u2029]/g, " ") + .replaceAll("://", ":/\u200b/"); + const who = (e: AuditInput) => userText(e.email ?? e.sub ?? "someone"); function securityMessage(e: AuditInput): Message { const lines = [ @@ -241,7 +244,7 @@ export function alerter({ ), ); if (e.event === "session_kill" || e.event === "user_signout") - lines.push(`User: ${e.user}`); + lines.push(`User: ${userText(e.user)}`); lines.push(new URL("/admin/audit", appUrl).href); return { title: `CertShell: ${SECURITY_TITLES[e.event]?.(e, hostName(e))}`, @@ -273,7 +276,7 @@ export function alerter({ >("SELECT name, email, sub FROM runs WHERE id = ?") .get(runId); return { - title: `CertShell: task "${run?.name}" failed on ${failed.length} of ${hosts.length} host${hosts.length === 1 ? "" : "s"}`, + title: `CertShell: task "${userText(run?.name)}" failed on ${failed.length} of ${hosts.length} host${hosts.length === 1 ? "" : "s"}`, body: [ `Run by ${who({ event: "", email: run?.email, sub: run?.sub })}.`, ...failed.map( diff --git a/apps/api/test/alerts.test.ts b/apps/api/test/alerts.test.ts index 4318991..bcbbd7a 100644 --- a/apps/api/test/alerts.test.ts +++ b/apps/api/test/alerts.test.ts @@ -333,7 +333,7 @@ test("terminal event: mentions and links in the email arrive defused", async () session: "s1", }); expect((await sent(1))[0]?.args[2]).toBe( - "CertShell: ‹!here› _@\u200bchannel [x]\u200b(https://evil.test) fake line opened root@h1", + "CertShell: ‹!here› _@\u200bchannel [x]\u200b(https:/\u200b/evil.test) fake line opened root@h1", ); }); @@ -390,13 +390,13 @@ test("task_failed: mentions and links in the task name and email arrive defused" const cookie = `${SESSION_COOKIE}=${createSession(ctx.db, { iss: "https://id.test", sub: "user-2", - email: "\n_@here", + email: "\n_@here\u2028https://evil.test", groups: ["certshell-admins"], })}`; const { id } = await post( "", { - name: " ", + name: " https://evil.test/login", script: "true", targets: [{ host: "h1", login: "root" }], }, @@ -405,10 +405,10 @@ test("task_failed: mentions and links in the task name and email arrive defused" const runId = (await post(`/${id}/run`, {}, cookie)).id; const [s] = await sent(1); expect(s?.args[2]).toBe( - 'CertShell: task "‹!channel› ‹https://evil.test|x›" failed on 1 of 1 host', + 'CertShell: task "‹!channel› ‹https:/\u200b/evil.test|x› https:/\u200b/evil.test/login" failed on 1 of 1 host', ); expect(s?.body).toBe( - `Run by ‹!channel› _@\u200bhere.\nroot@h1: exit 1\nhttps://ssh.test/tasks/runs/${runId}`, + `Run by ‹!channel› _@\u200bhere https:/\u200b/evil.test.\nroot@h1: exit 1\nhttps://ssh.test/tasks/runs/${runId}`, ); });