From 7c5bf1c675742a488402cd8d06c2c476a76145f2 Mon Sep 17 00:00:00 2001
From: Vincent Koc <vincentkoc@ieee.org>
Date: Thu, 30 Apr 2026 00:08:14 -0700
Subject: [PATCH] fix(security): remediate CodeQL alerts

---
 src/infra/outbound/sanitize-text.test.ts |  9 ++++
 src/infra/outbound/sanitize-text.ts      | 57 +++++++++++++-----------
 src/security/audit-extra.sync.test.ts    |  2 +
 src/security/secret-equal.ts             | 25 +++++++++--
 4 files changed, 65 insertions(+), 28 deletions(-)

diff --git a/src/infra/outbound/sanitize-text.test.ts b/src/infra/outbound/sanitize-text.test.ts
index 8842088b761..0f20844642c 100644
--- a/src/infra/outbound/sanitize-text.test.ts
+++ b/src/infra/outbound/sanitize-text.test.ts
@@ -63,6 +63,15 @@ describe("sanitizeForPlainText", () => {
     expect(sanitizeForPlainText('<a href="https://example.com">link</a>')).toBe("link");
   });
 
+  it("keeps stripping tags exposed by malformed tag text", () => {
+    const sanitized = sanitizeForPlainText(
+      "before <<script>script>alert(1)</<script>script> after",
+    );
+
+    expect(sanitized).toBe("before alert(1) after");
+    expect(sanitized).not.toContain("<script");
+  });
+
   it("strips known internal runtime scaffolding tags including underscore names", () => {
     expect(sanitizeForPlainText("ok <previous_response>null</previous_response> done")).toBe(
       "ok  done",
diff --git a/src/infra/outbound/sanitize-text.ts b/src/infra/outbound/sanitize-text.ts
index 9bb68b76051..64180786edb 100644
--- a/src/infra/outbound/sanitize-text.ts
+++ b/src/infra/outbound/sanitize-text.ts
@@ -25,6 +25,17 @@ const INTERNAL_RUNTIME_SCAFFOLDING_TAG_RE = new RegExp(
   `<\\s*\\/?\\s*(?:${INTERNAL_RUNTIME_SCAFFOLDING_TAG_PATTERN})\\b[^>]*>`,
   "gi",
 );
+const HTML_TAG_RE = /<\/?[a-z][a-z0-9_-]*\b[^>]*>/gi;
+
+function stripRemainingHtmlTags(text: string): string {
+  let previous: string;
+  let current = text;
+  do {
+    previous = current;
+    current = current.replace(HTML_TAG_RE, "");
+  } while (current !== previous);
+  return current;
+}
 
 export function stripInternalRuntimeScaffolding(text: string): string {
   return text
@@ -42,29 +53,25 @@ export function stripInternalRuntimeScaffolding(text: string): string {
  * prose (e.g. `a < b`).
  */
 export function sanitizeForPlainText(text: string): string {
-  return (
-    stripInternalRuntimeScaffolding(text)
-      // Preserve angle-bracket autolinks as plain URLs before tag stripping.
-      .replace(/<((?:https?:\/\/|mailto:)[^<>\s]+)>/gi, "$1")
-      // Line breaks
-      .replace(/<br\s*\/?>/gi, "\n")
-      // Block elements → newlines
-      .replace(/<\/?(p|div)>/gi, "\n")
-      // Bold → WhatsApp/Signal bold
-      .replace(/<(b|strong)>(.*?)<\/\1>/gi, "*$2*")
-      // Italic → WhatsApp/Signal italic
-      .replace(/<(i|em)>(.*?)<\/\1>/gi, "_$2_")
-      // Strikethrough → WhatsApp/Signal strikethrough
-      .replace(/<(s|strike|del)>(.*?)<\/\1>/gi, "~$2~")
-      // Inline code
-      .replace(/<code>(.*?)<\/code>/gi, "`$1`")
-      // Headings → bold text with newline
-      .replace(/<h[1-6][^>]*>(.*?)<\/h[1-6]>/gi, "\n*$1*\n")
-      // List items → bullet points
-      .replace(/<li[^>]*>(.*?)<\/li>/gi, "• $1\n")
-      // Strip remaining HTML tags (require tag-like structure: <word...>)
-      .replace(/<\/?[a-z][a-z0-9_-]*\b[^>]*>/gi, "")
-      // Collapse 3+ consecutive newlines into 2
-      .replace(/\n{3,}/g, "\n\n")
-  );
+  const converted = stripInternalRuntimeScaffolding(text)
+    // Preserve angle-bracket autolinks as plain URLs before tag stripping.
+    .replace(/<((?:https?:\/\/|mailto:)[^<>\s]+)>/gi, "$1")
+    // Line breaks
+    .replace(/<br\s*\/?>/gi, "\n")
+    // Block elements → newlines
+    .replace(/<\/?(p|div)>/gi, "\n")
+    // Bold → WhatsApp/Signal bold
+    .replace(/<(b|strong)>(.*?)<\/\1>/gi, "*$2*")
+    // Italic → WhatsApp/Signal italic
+    .replace(/<(i|em)>(.*?)<\/\1>/gi, "_$2_")
+    // Strikethrough → WhatsApp/Signal strikethrough
+    .replace(/<(s|strike|del)>(.*?)<\/\1>/gi, "~$2~")
+    // Inline code
+    .replace(/<code>(.*?)<\/code>/gi, "`$1`")
+    // Headings → bold text with newline
+    .replace(/<h[1-6][^>]*>(.*?)<\/h[1-6]>/gi, "\n*$1*\n")
+    // List items → bullet points
+    .replace(/<li[^>]*>(.*?)<\/li>/gi, "• $1\n");
+
+  return stripRemainingHtmlTags(converted).replace(/\n{3,}/g, "\n\n");
 }
diff --git a/src/security/audit-extra.sync.test.ts b/src/security/audit-extra.sync.test.ts
index e8dfc9bf73f..8d872eb32fb 100644
--- a/src/security/audit-extra.sync.test.ts
+++ b/src/security/audit-extra.sync.test.ts
@@ -52,6 +52,8 @@ describe("safeEqualSecret", () => {
     ["secret-token", "secret-token", true],
     ["secret-token", "secret-tokEn", false],
     ["short", "much-longer", false],
+    ["", "", true],
+    ["", "secret", false],
     [undefined, "secret", false],
     ["secret", undefined, false],
     [null, "secret", false],
diff --git a/src/security/secret-equal.ts b/src/security/secret-equal.ts
index 4e0e3ead29b..69c60ff30a4 100644
--- a/src/security/secret-equal.ts
+++ b/src/security/secret-equal.ts
@@ -1,4 +1,13 @@
-import { createHash, timingSafeEqual } from "node:crypto";
+import { timingSafeEqual } from "node:crypto";
+
+function padSecretBytes(bytes: Buffer, length: number): Buffer {
+  if (bytes.length === length) {
+    return bytes;
+  }
+  const padded = Buffer.alloc(length);
+  bytes.copy(padded);
+  return padded;
+}
 
 export function safeEqualSecret(
   provided: string | undefined | null,
@@ -7,6 +16,16 @@ export function safeEqualSecret(
   if (typeof provided !== "string" || typeof expected !== "string") {
     return false;
   }
-  const hash = (s: string) => createHash("sha256").update(s).digest();
-  return timingSafeEqual(hash(provided), hash(expected));
+  const providedBytes = Buffer.from(provided, "utf8");
+  const expectedBytes = Buffer.from(expected, "utf8");
+  const byteLength = Math.max(providedBytes.length, expectedBytes.length);
+  if (byteLength === 0) {
+    return true;
+  }
+  return (
+    timingSafeEqual(
+      padSecretBytes(providedBytes, byteLength),
+      padSecretBytes(expectedBytes, byteLength),
+    ) && providedBytes.length === expectedBytes.length
+  );
 }