fix(security): limit skill URL import reads

src/main/skill-github.ts

@@ -1,3 +1,5 @@
+import { readResponseWithLimit } from "./skills/read-response-with-limit.js";
+
 /**
  * Data returned for each skill found in a GitHub repository.
  */
@@ -24,44 +26,6 @@ export const MAX_TOTAL_BYTES = 32 * 1024 * 1024;
 /** 10 MB — upper bound for the recursive git tree JSON response. */
 const MAX_TREE_BYTES = 10 * 1024 * 1024;
 
-/**
- * Read a fetch Response body with a byte-size ceiling.
- * Throws if the body exceeds `maxBytes`.
- */
-async function readResponseWithLimit(response: Response, maxBytes: number): Promise<string> {
-  const contentLength = response.headers.get("Content-Length");
-  if (contentLength !== null && Number(contentLength) > maxBytes) {
-    throw new Error(`Response too large: ${contentLength} bytes (max ${maxBytes})`);
-  }
-
-  const reader = response.body?.getReader();
-  if (!reader) {
-    // Fallback: body not streamable (e.g. older Node fetch polyfill)
-    const text = await response.text();
-    if (text.length > maxBytes) {
-      throw new Error(`Response body too large: ${text.length} bytes (max ${maxBytes})`);
-    }
-    return text;
-  }
-
-  const decoder = new TextDecoder();
-  let totalBytes = 0;
-  let result = "";
-
-  for (;;) {
-    const { done, value } = await reader.read();
-    if (done) break;
-    totalBytes += value.byteLength;
-    if (totalBytes > maxBytes) {
-      await reader.cancel();
-      throw new Error(`Response body exceeded ${maxBytes} bytes`);
-    }
-    result += decoder.decode(value, { stream: true });
-  }
-  result += decoder.decode();
-  return result;
-}
-
 /**
  * Resolve the latest commit SHA for a given ref (branch, tag, or HEAD).
  * Returns the 40-character hex SHA on success, null on error.

src/main/skills/import-url.ts

@@ -3,7 +3,9 @@ import type { AppDatabase } from "../database";
 import type { FileInventoryEntry, SkillImportResult } from "../skill-types.js";
 import { parseFrontmatterMarkdown } from "../skill-format.js";
 import { deriveCanonicalSkillKey, normalizeSkillSlug } from "../skill-keys.js";
+import { MAX_SKILL_BYTES } from "../skill-github.js";
 import { materializeSkillFiles } from "./materialize";
+import { readResponseWithLimit } from "./read-response-with-limit.js";
 
 export interface UrlImportContext {
   db: AppDatabase;
@@ -26,7 +28,7 @@ export async function importFromUrl(
   if (!response.ok) {
     throw new Error(`Failed to fetch skill from URL: ${response.status} ${url}`);
   }
-  const markdown = await response.text();
+  const markdown = await readResponseWithLimit(response, MAX_SKILL_BYTES);
   const parsed = parseFrontmatterMarkdown(markdown);
   const nowTs = new Date().toISOString();
 

src/main/skills/read-response-with-limit.ts

@@ -0,0 +1,49 @@
+export async function readResponseWithLimit(response: Response, maxBytes: number): Promise<string> {
+  if (!Number.isSafeInteger(maxBytes) || maxBytes < 0) {
+    throw new Error(`Invalid response byte limit: ${maxBytes}`);
+  }
+
+  const contentLengthHeader = response.headers.get("Content-Length");
+  if (contentLengthHeader !== null) {
+    const contentLength = Number(contentLengthHeader);
+    if (!Number.isSafeInteger(contentLength) || contentLength < 0) {
+      throw new Error(`Invalid Content-Length header: ${contentLengthHeader}`);
+    }
+    if (contentLength > maxBytes) {
+      throw new Error(`Response too large: ${contentLength} bytes (max ${maxBytes})`);
+    }
+  }
+
+  const reader = response.body?.getReader();
+  if (!reader) {
+    const text = await response.text();
+    const byteLength = new TextEncoder().encode(text).byteLength;
+    if (byteLength > maxBytes) {
+      throw new Error(`Response body exceeded ${maxBytes} bytes`);
+    }
+    return text;
+  }
+
+  const chunks: Uint8Array[] = [];
+  let totalBytes = 0;
+
+  for (;;) {
+    const { done, value } = await reader.read();
+    if (done) break;
+    totalBytes += value.byteLength;
+    if (totalBytes > maxBytes) {
+      await reader.cancel();
+      throw new Error(`Response body exceeded ${maxBytes} bytes`);
+    }
+    chunks.push(value);
+  }
+
+  const body = new Uint8Array(totalBytes);
+  let offset = 0;
+  for (const chunk of chunks) {
+    body.set(chunk, offset);
+    offset += chunk.byteLength;
+  }
+
+  return new TextDecoder().decode(body);
+}

tests/unit/skill-response-limit.test.ts

@@ -0,0 +1,76 @@
+import { describe, expect, it } from "vitest";
+import { readResponseWithLimit } from "@main/skills/read-response-with-limit";
+
+const encoder = new TextEncoder();
+
+function responseFromChunks(chunks: string[], onCancel?: () => void): Response {
+  let index = 0;
+  const stream = new ReadableStream<Uint8Array>({
+    pull(controller) {
+      const chunk = chunks[index];
+      index += 1;
+      if (chunk === undefined) {
+        controller.close();
+        return;
+      }
+      controller.enqueue(encoder.encode(chunk));
+    },
+    cancel() {
+      onCancel?.();
+    },
+  });
+  return new Response(stream);
+}
+
+describe("readResponseWithLimit", () => {
+  it("returns text when the UTF-8 byte count matches the limit", async () => {
+    const input = "hello 世界";
+    const maxBytes = encoder.encode(input).byteLength;
+
+    await expect(readResponseWithLimit(new Response(input), maxBytes)).resolves.toBe(input);
+  });
+
+  it("rejects oversized Content-Length before body reads", async () => {
+    const response = new Response("small", {
+      headers: { "Content-Length": "6" },
+    });
+
+    await expect(readResponseWithLimit(response, 5)).rejects.toThrow(
+      "Response too large: 6 bytes (max 5)",
+    );
+  });
+
+  it("rejects malformed Content-Length headers", async () => {
+    const response = new Response("small", {
+      headers: { "Content-Length": "NaN" },
+    });
+
+    await expect(readResponseWithLimit(response, 5)).rejects.toThrow(
+      "Invalid Content-Length header: NaN",
+    );
+  });
+
+  it("cancels the stream when the body exceeds the byte limit while reading", async () => {
+    let cancelled = false;
+    const response = responseFromChunks(["abc", "def"], () => {
+      cancelled = true;
+    });
+
+    await expect(readResponseWithLimit(response, 5)).rejects.toThrow(
+      "Response body exceeded 5 bytes",
+    );
+    expect(cancelled).toBe(true);
+  });
+
+  it("keeps byte-limit checks for non-streamable Response objects", async () => {
+    const response = {
+      headers: new Headers(),
+      body: null,
+      text: async () => "世界",
+    } as Response;
+
+    await expect(readResponseWithLimit(response, 5)).rejects.toThrow(
+      "Response body exceeded 5 bytes",
+    );
+  });
+});

tests/unit/skill-url-import.test.ts

@@ -0,0 +1,108 @@
+import { existsSync } from "node:fs";
+import { mkdtemp, rm } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { AppDatabase } from "@main/database";
+import { MAX_SKILL_BYTES } from "@main/skill-github";
+import { importFromUrl } from "@main/skills/import-url";
+
+const encoder = new TextEncoder();
+
+function responseFromChunks(chunks: string[], onCancel: () => void): Response {
+  let index = 0;
+  return {
+    ok: true,
+    status: 200,
+    headers: new Headers(),
+    body: {
+      getReader() {
+        return {
+          async read() {
+            const chunk = chunks[index];
+            index += 1;
+            if (chunk === undefined) {
+              return { done: true, value: undefined };
+            }
+            return { done: false, value: encoder.encode(chunk) };
+          },
+          async cancel() {
+            onCancel();
+          },
+        };
+      },
+    },
+    async text() {
+      throw new Error("streaming URL imports should not call response.text()");
+    },
+  } as unknown as Response;
+}
+
+describe("skill URL import", () => {
+  let tempDir: string;
+  let db: AppDatabase;
+  let companyId: string;
+
+  beforeEach(async () => {
+    tempDir = await mkdtemp(join(tmpdir(), "agentcompany-skill-url-import-"));
+    db = new AppDatabase(tempDir);
+    db.init();
+    companyId = db.core.company.saveCompany({
+      name: "Skill Import Test",
+      description: "URL import limit coverage",
+      status: "active",
+    });
+  });
+
+  afterEach(async () => {
+    db.close();
+    await rm(tempDir, { recursive: true, force: true });
+  });
+
+  it("rejects URL imports before materializing oversized responses", async () => {
+    const safeFetch = vi.fn(async () => new Response("---\nname: Large\n---\n", {
+      headers: { "Content-Length": String(MAX_SKILL_BYTES + 1) },
+    }));
+
+    await expect(importFromUrl({
+      db,
+      logger: {
+        info: vi.fn(),
+        warn: vi.fn(),
+        error: vi.fn(),
+      },
+      skillFilesDir: (ownerCompanyId, slug) => join(tempDir, "skills", ownerCompanyId, slug),
+    }, companyId, "https://example.com/skills/large.md", safeFetch)).rejects.toThrow(
+      `Response too large: ${MAX_SKILL_BYTES + 1} bytes (max ${MAX_SKILL_BYTES})`,
+    );
+
+    expect(safeFetch).toHaveBeenCalledWith("https://example.com/skills/large.md");
+    expect(db.core.skills.listCompanySkills(companyId)).toEqual([]);
+  });
+
+  it("cancels chunked URL import reads that exceed the skill byte limit", async () => {
+    let cancelled = false;
+    const skillDir = join(tempDir, "skills", companyId, "large");
+    const header = "---\nname: Large\nslug: large\n---\n";
+    const firstChunk = "a".repeat(MAX_SKILL_BYTES - encoder.encode(header).byteLength);
+    const safeFetch = vi.fn(async () => responseFromChunks([header, firstChunk, "b"], () => {
+      cancelled = true;
+    }));
+
+    await expect(importFromUrl({
+      db,
+      logger: {
+        info: vi.fn(),
+        warn: vi.fn(),
+        error: vi.fn(),
+      },
+      skillFilesDir: (ownerCompanyId, slug) => join(tempDir, "skills", ownerCompanyId, slug),
+    }, companyId, "https://example.com/skills/large.md", safeFetch)).rejects.toThrow(
+      `Response body exceeded ${MAX_SKILL_BYTES} bytes`,
+    );
+
+    expect(cancelled).toBe(true);
+    expect(existsSync(skillDir)).toBe(false);
+    expect(db.core.skills.listCompanySkills(companyId)).toEqual([]);
+  });
+});