security: bound attacker-controlled scan fan-out and rate-limit write race

.gitignore

@@ -20,6 +20,7 @@ TRIAGE.json
 TRIAGE.md
 .triage-state/
 .threat-model-state/
+.security-scans/
 
 # CycloneDX SBOM is generated in CI per release and attached as a release
 # asset — never committed (release.yml builds it from the lockfile).

src/analyzers/mta-sts.ts

@@ -2,6 +2,12 @@ import { queryTxt } from "../dns/client.js";
 import type { ScanBudget } from "../dns/scan-budget.js";
 import type { MtaStsPolicy, MtaStsResult, Validation } from "./types.js";
 
+// Cap the fetched policy body before decoding it (GHSA-p676-gc7j-96mx). RFC 8461
+// policies are tiny, but an attacker who controls mta-sts.<domain> could
+// otherwise stream an unbounded body into the isolate. This mirrors the
+// MAX_BODY_BYTES ceiling that src/analyzers/security-txt.ts already applies.
+export const MAX_POLICY_BYTES = 64 * 1024;
+
 export async function analyzeMtaSts(
   domain: string,
   budget?: ScanBudget,
@@ -119,7 +125,18 @@ async function fetchPolicy(domain: string): Promise<MtaStsPolicy | null> {
     if ((resp.type as string) === "opaqueredirect") return null;
     if (!resp.ok) return null;
 
-    const text = await resp.text();
+    // Bound the body before decoding — reading via arrayBuffer()+slice caps
+    // memory regardless of a lying Content-Length or a slow infinite stream
+    // (the 3s AbortSignal bounds time, not bytes). Same control as security-txt.
+    const buffer = await resp.arrayBuffer();
+    const slice =
+      buffer.byteLength > MAX_POLICY_BYTES
+        ? buffer.slice(0, MAX_POLICY_BYTES)
+        : buffer;
+    const text = new TextDecoder("utf-8", {
+      fatal: false,
+      ignoreBOM: false,
+    }).decode(slice);
     return parsePolicy(text);
   } catch {
     return null;

src/index.ts

@@ -1453,12 +1453,27 @@ export { normalizeDomain };
 // is dropped silently — an invalid selector cannot match a real DKIM key.
 const VALID_SELECTOR = /^[A-Za-z0-9._-]+$/;
 
+// DoS guard (GHSA-6fqp-4vhc-59mf): every custom selector becomes one concurrent
+// DNS lookup in analyzeDkim, so an unbounded attacker-supplied list is a DNS
+// amplification vector charged against a single rate-limit token. Bound both
+// the per-item length (RFC 1035 label limit) and the count. 16 custom selectors
+// is generous given ~37 built-in COMMON_SELECTORS. parseSelectorsFromArray in
+// src/mcp/handler.ts mirrors these limits for the MCP path.
+export const MAX_SELECTOR_LENGTH = 63;
+export const MAX_SELECTORS = 16;
+
 export function parseSelectors(raw: string | undefined): string[] {
   if (!raw) return [];
   return raw
     .split(",")
     .map((s) => s.trim())
-    .filter((s) => s.length > 0 && VALID_SELECTOR.test(s));
+    .filter(
+      (s) =>
+        s.length > 0 &&
+        s.length <= MAX_SELECTOR_LENGTH &&
+        VALID_SELECTOR.test(s),
+    )
+    .slice(0, MAX_SELECTORS);
 }
 
 // Cron handler — runs nightly per the `[triggers] crons` entry in wrangler.toml.

src/mcp/handler.ts

@@ -11,8 +11,22 @@ import type { ScoringConfig } from "../shared/scoring.js";
 // DKIM selector charset per RFC 6376 §3.1 — mirrors VALID_SELECTOR in index.ts.
 const VALID_SELECTOR = /^[A-Za-z0-9._-]+$/;
 
-function parseSelectorsFromArray(raw: string[]): string[] {
-  return raw.filter((s) => s.length > 0 && VALID_SELECTOR.test(s));
+// Mirrors MAX_SELECTOR_LENGTH / MAX_SELECTORS in src/index.ts
+// (GHSA-6fqp-4vhc-59mf). Enforced server-side regardless of the inputSchema
+// maxItems/maxLength below, which are advisory only — handleToolCall never
+// validates arguments against the schema.
+const MAX_SELECTOR_LENGTH = 63;
+const MAX_SELECTORS = 16;
+
+export function parseSelectorsFromArray(raw: string[]): string[] {
+  return raw
+    .filter(
+      (s) =>
+        s.length > 0 &&
+        s.length <= MAX_SELECTOR_LENGTH &&
+        VALID_SELECTOR.test(s),
+    )
+    .slice(0, MAX_SELECTORS);
 }
 
 export const MCP_PROTOCOL_VERSION = "2025-03-26";
@@ -46,7 +60,12 @@ const SCAN_DOMAIN_TOOL = {
       },
       dkim_selectors: {
         type: "array",
-        items: { type: "string", pattern: "^[A-Za-z0-9._-]+$" },
+        maxItems: MAX_SELECTORS,
+        items: {
+          type: "string",
+          pattern: "^[A-Za-z0-9._-]+$",
+          maxLength: MAX_SELECTOR_LENGTH,
+        },
         description:
           "Extra DKIM selectors to probe beyond the built-in defaults.",
       },

test/index.test.ts

@@ -1,6 +1,12 @@
 import { beforeEach, describe, expect, it, vi } from "vitest";
 import { COMMON_SELECTORS } from "../src/analyzers/dkim.js";
-import { app, normalizeDomain, parseSelectors } from "../src/index.js";
+import {
+  app,
+  MAX_SELECTOR_LENGTH,
+  MAX_SELECTORS,
+  normalizeDomain,
+  parseSelectors,
+} from "../src/index.js";
 import { _memoryStore } from "../src/rate-limit.js";
 import { LEARN_SIBLINGS } from "../src/views/learn.js";
 
@@ -124,6 +130,21 @@ describe("parseSelectors", () => {
   it("returns single selector", () => {
     expect(parseSelectors("google")).toEqual(["google"]);
   });
+
+  // f27 / GHSA-6fqp-4vhc-59mf — attacker-controlled selector lists must not
+  // fan out into one DNS lookup each with no ceiling.
+  it("caps the number of selectors at MAX_SELECTORS", () => {
+    const many = Array.from({ length: 500 }, (_, i) => `s${i}`).join(",");
+    const result = parseSelectors(many);
+    expect(result.length).toBe(MAX_SELECTORS);
+    expect(result[0]).toBe("s0"); // keeps the first N, deterministically
+  });
+
+  it("drops selectors longer than MAX_SELECTOR_LENGTH", () => {
+    const tooLong = "a".repeat(MAX_SELECTOR_LENGTH + 1);
+    const ok = "b".repeat(MAX_SELECTOR_LENGTH);
+    expect(parseSelectors(`${tooLong},${ok},google`)).toEqual([ok, "google"]);
+  });
 });
 
 describe("normalizeDomain — extended edge cases", () => {

test/mcp.test.ts

@@ -1,4 +1,5 @@
 import { describe, expect, it, vi } from "vitest";
+import { MAX_SELECTORS } from "../src/index.js";
 import {
   handleMcpRequest,
   MCP_PROTOCOL_VERSION,
@@ -214,6 +215,26 @@ describe("handleMcpRequest — tools/call", () => {
     expect(vi.mocked(scan)).toHaveBeenCalled();
   });
 
+  // f27 / GHSA-6fqp-4vhc-59mf — the MCP path must enforce the selector count
+  // cap server-side (the inputSchema maxItems is advisory only).
+  it("caps dkim_selectors at MAX_SELECTORS before calling scan", async () => {
+    const { scan } = await import("../src/orchestrator.js");
+    vi.mocked(scan).mockClear();
+    const many = Array.from({ length: 500 }, (_, i) => `s${i}`);
+    const { json } = await rpc({
+      jsonrpc: "2.0",
+      id: 71,
+      method: "tools/call",
+      params: {
+        name: "scan_domain",
+        arguments: { domain: "example.com", dkim_selectors: many },
+      },
+    });
+    expect((json.result as { isError: boolean }).isError).toBe(false);
+    const selectorsArg = vi.mocked(scan).mock.calls[0]?.[1] as string[];
+    expect(selectorsArg.length).toBe(MAX_SELECTORS);
+  });
+
   it("preserves JSON-RPC id across the full call", async () => {
     const { json } = await rpc({
       jsonrpc: "2.0",

test/mta-sts.test.ts

@@ -5,7 +5,7 @@ vi.mock("../src/dns/client.js", () => ({
   queryMx: vi.fn(),
 }));
 
-import { analyzeMtaSts } from "../src/analyzers/mta-sts.js";
+import { analyzeMtaSts, MAX_POLICY_BYTES } from "../src/analyzers/mta-sts.js";
 import { queryTxt } from "../src/dns/client.js";
 
 const mockQueryTxt = vi.mocked(queryTxt);
@@ -15,11 +15,22 @@ beforeEach(() => {
   vi.restoreAllMocks();
 });
 
+// fetchPolicy reads the body via resp.arrayBuffer() (size-capped, mirroring
+// security-txt) — so the mock must expose arrayBuffer, not just text.
 function mockFetchPolicy(body: string | null, ok = true) {
   vi.spyOn(globalThis, "fetch").mockResolvedValue(
     body === null
-      ? ({ ok: false, text: async () => "" } as Response)
-      : ({ ok, text: async () => body } as Response),
+      ? ({
+          ok: false,
+          text: async () => "",
+          arrayBuffer: async () => new ArrayBuffer(0),
+        } as Response)
+      : ({
+          ok,
+          text: async () => body,
+          arrayBuffer: async () =>
+            new TextEncoder().encode(body).buffer as ArrayBuffer,
+        } as Response),
   );
 }
 
@@ -57,6 +68,28 @@ describe("analyzeMtaSts", () => {
     ).toBe(true);
   });
 
+  // f29 / GHSA-p676-gc7j-96mx — an attacker who controls mta-sts.<domain> must
+  // not be able to stream an unbounded body into memory. The fetch body is
+  // capped at MAX_POLICY_BYTES; anything past the cap is never parsed.
+  it("caps the policy body and ignores content beyond MAX_POLICY_BYTES", async () => {
+    mockQueryTxt.mockResolvedValue({
+      entries: ["v=STSv1; id=20240101"],
+      raw: "v=STSv1; id=20240101",
+    });
+    // Valid directives in the first <64KB; a sentinel mx line pushed past the
+    // cap by a giant colon-less (therefore ignored-by-parser) filler line.
+    const prefix =
+      "version: STSv1\nmode: enforce\nmx: legit.example.com\nmax_age: 86400\n";
+    const filler = `${"#".repeat(MAX_POLICY_BYTES)}\n`;
+    const sentinel = "mx: sneaky.evil.example\n";
+    mockFetchPolicy(prefix + filler + sentinel);
+
+    const result = await analyzeMtaSts("example.com");
+    expect(result.policy?.mx).toContain("legit.example.com");
+    // The sentinel sits beyond the byte cap, so it must never reach the parser.
+    expect(result.policy?.mx).not.toContain("sneaky.evil.example");
+  });
+
   it("passes when DNS record found with v=STSv1", async () => {
     mockQueryTxt.mockResolvedValue({
       entries: ["v=STSv1; id=20240101"],