diff --git a/src/main/skill-github.ts b/src/main/skill-github.ts index ab2feb39..f267b88c 100644 --- a/src/main/skill-github.ts +++ b/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 { - 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. diff --git a/src/main/skills/import-url.ts b/src/main/skills/import-url.ts index be8071e9..d90e03aa 100644 --- a/src/main/skills/import-url.ts +++ b/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(); diff --git a/src/main/skills/read-response-with-limit.ts b/src/main/skills/read-response-with-limit.ts new file mode 100644 index 00000000..9233b680 --- /dev/null +++ b/src/main/skills/read-response-with-limit.ts @@ -0,0 +1,49 @@ +export async function readResponseWithLimit(response: Response, maxBytes: number): Promise { + 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); +} diff --git a/tests/unit/skill-response-limit.test.ts b/tests/unit/skill-response-limit.test.ts new file mode 100644 index 00000000..7cbc7618 --- /dev/null +++ b/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({ + 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", + ); + }); +}); diff --git a/tests/unit/skill-url-import.test.ts b/tests/unit/skill-url-import.test.ts new file mode 100644 index 00000000..87855d13 --- /dev/null +++ b/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([]); + }); +});