diff --git a/src/main/skill-github.ts b/src/main/skill-github.ts index f267b88c..64db9683 100644 --- a/src/main/skill-github.ts +++ b/src/main/skill-github.ts @@ -1,4 +1,10 @@ -import { readResponseWithLimit } from "./skills/read-response-with-limit.js"; +import { readResponseBytesWithLimit, readResponseWithLimit } from "./skills/read-response-with-limit.js"; + +export interface GitHubSkillFileData { + path: string; + content: Uint8Array; + mode: string | null; +} /** * Data returned for each skill found in a GitHub repository. @@ -6,7 +12,7 @@ import { readResponseWithLimit } from "./skills/read-response-with-limit.js"; export interface GitHubSkillData { skillMdPath: string; markdown: string; - inventory: string[]; + files: GitHubSkillFileData[]; owner: string; repo: string; } @@ -20,12 +26,44 @@ interface Logger { /** 2 MB — generous upper bound for a single SKILL.md (markdown) file. */ export const MAX_SKILL_BYTES = 2 * 1024 * 1024; -/** 32 MB — aggregate upper bound for all SKILL.md content from a single skill pack. */ +/** 8 MB — upper bound for a single non-SKILL.md package file. */ +export const MAX_SKILL_FILE_BYTES = 8 * 1024 * 1024; + +/** 32 MB — aggregate upper bound for all materialized files from a skill pack. */ 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; +interface GitHubTreeEntry { + path: string; + type: string; + mode?: string; +} + +function encodeGitHubBlobPath(filePath: string): string { + if (filePath.includes("\0") || filePath.includes("\\") || filePath.startsWith("/") || /^[A-Za-z]:/.test(filePath)) { + throw new Error(`Invalid GitHub skill file path: ${filePath}`); + } + const parts = filePath.split("/"); + if (parts.some((part) => part.length === 0 || part === "." || part === "..")) { + throw new Error(`Invalid GitHub skill file path: ${filePath}`); + } + return parts.map((part) => encodeURIComponent(part)).join("/"); +} + +function rawGitHubUrl(owner: string, repo: string, ref: string, filePath: string): string { + return `https://raw.githubusercontent.com/${owner}/${repo}/${ref}/${encodeGitHubBlobPath(filePath)}`; +} + +function assertTotalSkillBytes(totalBytes: number): void { + if (totalBytes > MAX_TOTAL_BYTES) { + throw new Error( + `Skill pack exceeded aggregate limit of ${MAX_TOTAL_BYTES} bytes`, + ); + } +} + /** * Resolve the latest commit SHA for a given ref (branch, tag, or HEAD). * Returns the 40-character hex SHA on success, null on error. @@ -87,7 +125,7 @@ export async function fetchGitHubSkills( return { skills: [], commitSha: ref }; } const treeData = JSON.parse(await readResponseWithLimit(treeResponse, MAX_TREE_BYTES)) as { - tree?: Array<{ path: string; type: string }>; + tree?: GitHubTreeEntry[]; sha?: string; }; const treeSha = typeof treeData.sha === "string" ? treeData.sha : ref; @@ -120,14 +158,15 @@ export async function fetchGitHubSkills( for (const skillMdPath of skillMdPaths) { try { + let skillBytes = 0; // Download content via raw.githubusercontent.com (no API rate limit) - const rawUrl = `https://raw.githubusercontent.com/${owner}/${repo}/${ref}/${skillMdPath}`; + const rawUrl = rawGitHubUrl(owner, repo, ref, skillMdPath); const contentResponse = await fetch(rawUrl); if (!contentResponse.ok) { logger?.warn(`Failed to download ${rawUrl}: ${contentResponse.status}`); continue; } - const remainingBudget = MAX_TOTAL_BYTES - totalBytes; + const remainingBudget = MAX_TOTAL_BYTES - totalBytes - skillBytes; const perFileCap = Math.min(MAX_SKILL_BYTES, Math.max(0, remainingBudget)); if (perFileCap === 0) { throw new Error( @@ -135,12 +174,8 @@ export async function fetchGitHubSkills( ); } const markdown = await readResponseWithLimit(contentResponse, perFileCap); - totalBytes += Buffer.byteLength(markdown, "utf-8"); - if (totalBytes > MAX_TOTAL_BYTES) { - throw new Error( - `Skill pack exceeded aggregate limit of ${MAX_TOTAL_BYTES} bytes`, - ); - } + skillBytes += Buffer.byteLength(markdown, "utf-8"); + assertTotalSkillBytes(totalBytes + skillBytes); // Collect sibling files in the same directory (or subdirectories) const skillDir = @@ -148,17 +183,42 @@ export async function fetchGitHubSkills( ? "" : skillMdPath.slice(0, skillMdPath.lastIndexOf("/")); const prefix = skillDir ? `${skillDir}/` : ""; - const inventory = allPaths.filter( - (p) => p !== skillMdPath && (prefix ? p.startsWith(prefix) : !p.includes("/")), + const inventoryEntries = allEntries.filter( + (entry) => entry.path !== skillMdPath && (prefix ? entry.path.startsWith(prefix) : !entry.path.includes("/")), ); + const files: GitHubSkillFileData[] = []; + + for (const entry of inventoryEntries) { + const fileRemainingBudget = MAX_TOTAL_BYTES - totalBytes - skillBytes; + const fileCap = Math.min(MAX_SKILL_FILE_BYTES, Math.max(0, fileRemainingBudget)); + if (fileCap === 0) { + throw new Error( + `Skill pack exceeded aggregate limit of ${MAX_TOTAL_BYTES} bytes`, + ); + } + const fileUrl = rawGitHubUrl(owner, repo, ref, entry.path); + const fileResponse = await fetch(fileUrl); + if (!fileResponse.ok) { + throw new Error(`Failed to download ${fileUrl}: ${fileResponse.status}`); + } + const content = await readResponseBytesWithLimit(fileResponse, fileCap); + skillBytes += content.byteLength; + assertTotalSkillBytes(totalBytes + skillBytes); + files.push({ + path: entry.path, + content, + mode: entry.mode ?? null, + }); + } skills.push({ skillMdPath, markdown, - inventory, + files, owner, - repo, + repo, }); + totalBytes += skillBytes; } catch (err) { logger?.error(`Error downloading skill ${skillMdPath}:`, err); continue; diff --git a/src/main/skills/discovery.ts b/src/main/skills/discovery.ts index 7c382c94..650c9ad6 100644 --- a/src/main/skills/discovery.ts +++ b/src/main/skills/discovery.ts @@ -1,4 +1,4 @@ -import { readdirSync, realpathSync, type Dirent } from "node:fs"; +import { readdirSync, realpathSync, statSync, type Dirent } from "node:fs"; import { join, resolve, relative } from "node:path"; import type { FileInventoryEntry } from "../skill-types.js"; import { classifyInventoryKind } from "../skill-format.js"; @@ -133,10 +133,18 @@ export function buildLocalInventory( if (!entry.isFile()) continue; const resolvedPath = resolve(fullPath); if (resolvedPath === skillMdPath) continue; + let fileStat; + try { + fileStat = statSync(resolvedPath); + } catch (err) { + logger.warn(`[skill-import] statSync failed for ${resolvedPath}:`, err instanceof Error ? err.message : String(err)); + continue; + } + if (!fileStat.isFile()) continue; const relativePath = relative(skillDir, resolvedPath).replace(/\\/g, "/"); inventory.push({ path: relativePath, - kind: classifyInventoryKind(relativePath), + kind: (fileStat.mode & 0o111) !== 0 ? "script" : classifyInventoryKind(relativePath), }); } } diff --git a/src/main/skills/import-github.ts b/src/main/skills/import-github.ts index 9810c2b4..cd8c3117 100644 --- a/src/main/skills/import-github.ts +++ b/src/main/skills/import-github.ts @@ -1,6 +1,5 @@ import { randomUUID } from "node:crypto"; -import { mkdir, writeFile } from "node:fs/promises"; -import { join, basename, dirname } from "node:path"; +import { basename, dirname } from "node:path"; import type { AppDatabase } from "../database"; import type { FileInventoryEntry, SkillImportResult, SkillSourceType } from "../skill-types.js"; import { @@ -15,9 +14,8 @@ import { import { resolveGitHubCommitSha, fetchGitHubSkills, - MAX_SKILL_BYTES, - MAX_TOTAL_BYTES, } from "../skill-github.js"; +import { materializeSkillFiles, type MaterializedSkillFile } from "./materialize"; interface GitHubImportDeps { db: AppDatabase; @@ -45,23 +43,6 @@ export function createGitHubImporter({ db, logger, skillFilesDir }: GitHubImport } } - async function materializeSkillFiles( - companyId: string, - slug: string, - markdown: string, - _inventory: FileInventoryEntry[], - ): Promise { - if (markdown.length * 2 > MAX_SKILL_BYTES) { - throw new Error( - `SKILL.md exceeds ${MAX_SKILL_BYTES}-byte limit`, - ); - } - const dir = skillFilesDir(companyId, slug); - await mkdir(dir, { recursive: true }); - await writeFile(join(dir, "SKILL.md"), markdown, "utf-8"); - return dir; - } - async function importFromGitHub( companyId: string, url: string, @@ -90,16 +71,8 @@ export function createGitHubImporter({ db, logger, skillFilesDir }: GitHubImport const results: SkillImportResult[] = []; const sourceType: SkillSourceType = originalSkillsShUrl ? "skills_sh" : "github"; const nowTs = new Date().toISOString(); - let totalMaterializedBytes = 0; for (const ghSkill of githubSkills) { - const skillBytes = Buffer.byteLength(ghSkill.markdown, "utf-8"); - totalMaterializedBytes += skillBytes; - if (totalMaterializedBytes > MAX_TOTAL_BYTES) { - throw new Error( - `Skill pack exceeded aggregate limit of ${MAX_TOTAL_BYTES} bytes`, - ); - } const parsed = parseFrontmatterMarkdown(ghSkill.markdown); const rawSlug = normalizeSkillSlug(parsed.frontmatter.slug as string | undefined) ?? @@ -115,16 +88,18 @@ export function createGitHubImporter({ db, logger, skillFilesDir }: GitHubImport ? "" : ghSkill.skillMdPath.slice(0, ghSkill.skillMdPath.lastIndexOf("/")); const prefix = skillDir ? `${skillDir}/` : ""; - const inventory: FileInventoryEntry[] = [ - { path: "SKILL.md", kind: "skill" as const }, - ...ghSkill.inventory.map((filePath) => { - const relativePath = prefix ? filePath.slice(prefix.length) : filePath; + const materializedFiles: MaterializedSkillFile[] = [ + { path: "SKILL.md", kind: "skill", content: ghSkill.markdown }, + ...ghSkill.files.map((file) => { + const relativePath = prefix ? file.path.slice(prefix.length) : file.path; return { path: relativePath, - kind: classifyInventoryKind(relativePath), + kind: file.mode === "100755" ? "script" as const : classifyInventoryKind(relativePath), + content: file.content, }; }), ]; + const inventory: FileInventoryEntry[] = materializedFiles.map(({ path, kind }) => ({ path, kind })); const metadata: Record = { sourceKind: "github", @@ -153,7 +128,7 @@ export function createGitHubImporter({ db, logger, skillFilesDir }: GitHubImport const skillId = randomUUID(); - await materializeSkillFiles(companyId, rawSlug, ghSkill.markdown, inventory); + await materializeSkillFiles(skillFilesDir(companyId, rawSlug), materializedFiles); const { isUpdate } = db.core.skills.upsertCompanySkill({ id: skillId, diff --git a/src/main/skills/import-local.ts b/src/main/skills/import-local.ts index 9963b179..cfde352d 100644 --- a/src/main/skills/import-local.ts +++ b/src/main/skills/import-local.ts @@ -1,6 +1,6 @@ import { randomUUID } from "node:crypto"; import { existsSync, readFileSync, realpathSync, statSync } from "node:fs"; -import { resolve, basename, dirname } from "node:path"; +import { resolve, basename, dirname, isAbsolute, relative } from "node:path"; import type { AppDatabase } from "../database"; import type { FileInventoryEntry, SkillImportResult } from "../skill-types.js"; import { parseFrontmatterMarkdown, deriveTrustLevel } from "../skill-format.js"; @@ -14,7 +14,8 @@ import { findSkillMdFiles, buildLocalInventory, } from "./discovery"; -import { materializeSkillFiles } from "./materialize"; +import { materializeSkillFiles, normalizeMaterializedRelativePath, type MaterializedSkillFile } from "./materialize"; +import { MAX_SKILL_BYTES, MAX_SKILL_FILE_BYTES, MAX_TOTAL_BYTES } from "../skill-github.js"; export interface LocalImportContext { db: AppDatabase; @@ -26,6 +27,70 @@ export interface LocalImportContext { skillFilesDir: (companyId: string, slug: string) => string; } +function assertLocalFileSize(filePath: string, size: number, maxBytes: number): void { + if (size > maxBytes) { + throw new Error(`${filePath} exceeds ${maxBytes}-byte limit`); + } +} + +function assertLocalPackageSize(totalBytes: number): void { + if (totalBytes > MAX_TOTAL_BYTES) { + throw new Error(`Skill package exceeded aggregate limit of ${MAX_TOTAL_BYTES} bytes`); + } +} + +function readLocalSkillMarkdown(skillMdPath: string): string { + const stat = statSync(skillMdPath); + if (!stat.isFile()) { + throw new Error(`Local skill path is not a file: ${skillMdPath}`); + } + assertLocalFileSize(skillMdPath, stat.size, MAX_SKILL_BYTES); + return readFileSync(skillMdPath, "utf-8"); +} + +function assertPathInsideRoot(rootDir: string, targetPath: string, relativePath: string): void { + const relativePathFromRoot = relative(rootDir, targetPath); + if (relativePathFromRoot.length === 0 || relativePathFromRoot.startsWith("..") || isAbsolute(relativePathFromRoot)) { + throw new Error(`Skill inventory path escapes source directory: ${relativePath}`); + } +} + +function buildLocalMaterializedFiles( + skillDir: string, + markdown: string, + markdownBytes: number, + inventory: FileInventoryEntry[], +): MaterializedSkillFile[] { + const realSkillDir = realpathSync(skillDir); + const files: MaterializedSkillFile[] = [ + { path: "SKILL.md", kind: "skill", content: markdown }, + ]; + let totalBytes = markdownBytes; + assertLocalPackageSize(totalBytes); + + for (const entry of inventory) { + if (entry.path === "SKILL.md") continue; + const relativePath = normalizeMaterializedRelativePath(entry.path); + const sourcePath = resolve(realSkillDir, ...relativePath.split("/")); + const realSourcePath = realpathSync(sourcePath); + assertPathInsideRoot(realSkillDir, realSourcePath, relativePath); + const stat = statSync(realSourcePath); + if (!stat.isFile()) { + throw new Error(`Skill inventory path is not a file: ${relativePath}`); + } + assertLocalFileSize(relativePath, stat.size, MAX_SKILL_FILE_BYTES); + totalBytes += stat.size; + assertLocalPackageSize(totalBytes); + files.push({ + path: relativePath, + kind: entry.kind, + content: readFileSync(realSourcePath), + }); + } + + return files; +} + export async function importFromLocal( ctx: LocalImportContext, companyId: string, @@ -47,7 +112,8 @@ export async function importFromLocal( const nowTs = new Date().toISOString(); if (stat.isFile()) { - const markdown = readFileSync(resolvedPath, "utf-8"); + const markdown = readLocalSkillMarkdown(resolvedPath); + assertLocalPackageSize(Buffer.byteLength(markdown, "utf-8")); const parsed = parseFrontmatterMarkdown(markdown); const rawSlug = normalizeSkillSlug(parsed.frontmatter.slug as string | undefined) ?? @@ -67,7 +133,9 @@ export async function importFromLocal( }); const skillId = randomUUID(); - await materializeSkillFiles(skillFilesDir(companyId, rawSlug), markdown, inventory); + await materializeSkillFiles(skillFilesDir(companyId, rawSlug), [ + { path: "SKILL.md", kind: "skill", content: markdown }, + ]); const { isUpdate } = db.core.skills.upsertCompanySkill({ id: skillId, @@ -100,7 +168,8 @@ export async function importFromLocal( for (const skillMdPath of skillMdFiles) { const skillDir = dirname(skillMdPath); - const markdown = readFileSync(skillMdPath, "utf-8"); + const markdown = readLocalSkillMarkdown(skillMdPath); + const markdownBytes = Buffer.byteLength(markdown, "utf-8"); const parsed = parseFrontmatterMarkdown(markdown); const rawSlug = normalizeSkillSlug(parsed.frontmatter.slug as string | undefined) ?? @@ -119,7 +188,10 @@ export async function importFromLocal( }); const skillId = randomUUID(); - await materializeSkillFiles(skillFilesDir(companyId, rawSlug), markdown, inventory); + await materializeSkillFiles( + skillFilesDir(companyId, rawSlug), + buildLocalMaterializedFiles(skillDir, markdown, markdownBytes, inventory), + ); const { isUpdate } = db.core.skills.upsertCompanySkill({ id: skillId, diff --git a/src/main/skills/import-url.ts b/src/main/skills/import-url.ts index d90e03aa..bd0d9479 100644 --- a/src/main/skills/import-url.ts +++ b/src/main/skills/import-url.ts @@ -59,7 +59,9 @@ export async function importFromUrl( }); const skillId = randomUUID(); - await materializeSkillFiles(skillFilesDir(companyId, rawSlug), markdown, inventory); + await materializeSkillFiles(skillFilesDir(companyId, rawSlug), [ + { path: "SKILL.md", kind: "skill", content: markdown }, + ]); const { isUpdate } = db.core.skills.upsertCompanySkill({ id: skillId, diff --git a/src/main/skills/materialize.ts b/src/main/skills/materialize.ts index 5ee2caf0..9f8b1f90 100644 --- a/src/main/skills/materialize.ts +++ b/src/main/skills/materialize.ts @@ -1,19 +1,142 @@ import { existsSync } from "node:fs"; -import { mkdir, writeFile, rm } from "node:fs/promises"; -import { join } from "node:path"; +import { mkdir, rename, writeFile, rm } from "node:fs/promises"; +import { randomUUID } from "node:crypto"; +import { basename, dirname, isAbsolute, join, relative, resolve } from "node:path"; import type { FileInventoryEntry } from "../skill-types.js"; -import { MAX_SKILL_BYTES } from "../skill-github.js"; +import { MAX_SKILL_BYTES, MAX_SKILL_FILE_BYTES, MAX_TOTAL_BYTES } from "../skill-github.js"; + +export interface MaterializedSkillFile extends FileInventoryEntry { + content: string | Uint8Array; +} + +interface PreparedSkillFile extends FileInventoryEntry { + content: Uint8Array; +} + +const MATERIALIZED_FILE_MODE = 0o600; +const MATERIALIZED_SCRIPT_MODE = 0o700; +const MATERIALIZED_DIR_MODE = 0o700; + +export function normalizeMaterializedRelativePath(filePath: string): string { + if (filePath.includes("\0")) { + throw new Error("Skill file path contains a null byte"); + } + const normalized = filePath.replace(/\\/g, "/"); + if (normalized !== filePath) { + throw new Error(`Skill file path must use POSIX separators: ${filePath}`); + } + if (normalized.length === 0 || normalized.startsWith("/") || /^[A-Za-z]:/.test(normalized)) { + throw new Error(`Skill file path must be relative: ${filePath}`); + } + const parts = normalized.split("/"); + if (parts.some((part) => part.length === 0 || part === "." || part === "..")) { + throw new Error(`Skill file path contains traversal segments: ${filePath}`); + } + return parts.join("/"); +} + +function encodeContent(content: string | Uint8Array): Uint8Array { + if (typeof content === "string") { + return Buffer.from(content, "utf-8"); + } + return content; +} + +function ensureTargetInsideRoot(rootDir: string, targetPath: string, relativePath: string): void { + const root = resolve(rootDir); + const targetRelative = relative(root, targetPath); + if (targetRelative.length === 0 || targetRelative.startsWith("..") || isAbsolute(targetRelative)) { + throw new Error(`Skill file path escapes materialization root: ${relativePath}`); + } +} + +function prepareMaterializedFiles(files: MaterializedSkillFile[]): PreparedSkillFile[] { + const seen = new Set(); + const prepared: PreparedSkillFile[] = []; + let totalBytes = 0; + let hasRootSkill = false; + + for (const file of files) { + const path = normalizeMaterializedRelativePath(file.path); + const duplicateKey = process.platform === "win32" ? path.toLowerCase() : path; + if (seen.has(duplicateKey)) { + throw new Error(`Duplicate skill file path: ${path}`); + } + seen.add(duplicateKey); + + const content = encodeContent(file.content); + const maxBytes = path === "SKILL.md" ? MAX_SKILL_BYTES : MAX_SKILL_FILE_BYTES; + if (content.byteLength > maxBytes) { + throw new Error(`${path} exceeds ${maxBytes}-byte limit`); + } + totalBytes += content.byteLength; + if (totalBytes > MAX_TOTAL_BYTES) { + throw new Error(`Skill package exceeded aggregate limit of ${MAX_TOTAL_BYTES} bytes`); + } + if (path === "SKILL.md") { + hasRootSkill = true; + } + prepared.push({ path, kind: file.kind, content }); + } + + if (!hasRootSkill) { + throw new Error("Skill package is missing SKILL.md"); + } + return prepared; +} + +async function replaceDirectoryWithFiles(dir: string, files: PreparedSkillFile[]): Promise { + const parentDir = dirname(dir); + const dirName = basename(dir); + const stagingDir = join(parentDir, `.${dirName}.${randomUUID()}.tmp`); + let backupDir: string | null = null; + + await mkdir(parentDir, { recursive: true, mode: MATERIALIZED_DIR_MODE }); + await mkdir(stagingDir, { recursive: true, mode: MATERIALIZED_DIR_MODE }); + + try { + for (const file of files) { + const target = resolve(stagingDir, ...file.path.split("/")); + ensureTargetInsideRoot(stagingDir, target, file.path); + await mkdir(dirname(target), { recursive: true, mode: MATERIALIZED_DIR_MODE }); + await writeFile(target, file.content, { + mode: file.kind === "script" ? MATERIALIZED_SCRIPT_MODE : MATERIALIZED_FILE_MODE, + }); + } + + if (existsSync(dir)) { + backupDir = join(parentDir, `.${dirName}.${randomUUID()}.old`); + await rename(dir, backupDir); + } + + try { + await rename(stagingDir, dir); + } catch (error) { + if (backupDir && existsSync(backupDir)) { + await rename(backupDir, dir); + backupDir = null; + } + throw error; + } + + if (backupDir) { + await rm(backupDir, { recursive: true, force: true }); + backupDir = null; + } + } finally { + await rm(stagingDir, { recursive: true, force: true }); + if (backupDir) { + await rm(backupDir, { recursive: true, force: true }); + } + } +} export async function materializeSkillFiles( dir: string, - markdown: string, - _inventory: FileInventoryEntry[], + files: MaterializedSkillFile[], ): Promise { - if (markdown.length * 2 > MAX_SKILL_BYTES) { - throw new Error(`SKILL.md exceeds ${MAX_SKILL_BYTES}-byte limit`); - } - await mkdir(dir, { recursive: true }); - await writeFile(join(dir, "SKILL.md"), markdown, "utf-8"); + const prepared = prepareMaterializedFiles(files); + await replaceDirectoryWithFiles(dir, prepared); return dir; } diff --git a/src/main/skills/read-response-with-limit.ts b/src/main/skills/read-response-with-limit.ts index 9233b680..bee93ffb 100644 --- a/src/main/skills/read-response-with-limit.ts +++ b/src/main/skills/read-response-with-limit.ts @@ -1,8 +1,13 @@ -export async function readResponseWithLimit(response: Response, maxBytes: number): Promise { +const encoder = new TextEncoder(); +const decoder = new TextDecoder(); + +function assertValidByteLimit(maxBytes: number): void { if (!Number.isSafeInteger(maxBytes) || maxBytes < 0) { throw new Error(`Invalid response byte limit: ${maxBytes}`); } +} +function validateContentLength(response: Response, maxBytes: number): void { const contentLengthHeader = response.headers.get("Content-Length"); if (contentLengthHeader !== null) { const contentLength = Number(contentLengthHeader); @@ -13,15 +18,29 @@ export async function readResponseWithLimit(response: Response, maxBytes: number throw new Error(`Response too large: ${contentLength} bytes (max ${maxBytes})`); } } +} + +function assertWithinLimit(byteLength: number, maxBytes: number): void { + if (byteLength > maxBytes) { + throw new Error(`Response body exceeded ${maxBytes} bytes`); + } +} + +export async function readResponseBytesWithLimit(response: Response, maxBytes: number): Promise { + assertValidByteLimit(maxBytes); + validateContentLength(response, 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`); + if (typeof response.arrayBuffer === "function") { + const bytes = new Uint8Array(await response.arrayBuffer()); + assertWithinLimit(bytes.byteLength, maxBytes); + return bytes; } - return text; + const text = await response.text(); + const bytes = encoder.encode(text); + assertWithinLimit(bytes.byteLength, maxBytes); + return bytes; } const chunks: Uint8Array[] = []; @@ -33,7 +52,7 @@ export async function readResponseWithLimit(response: Response, maxBytes: number totalBytes += value.byteLength; if (totalBytes > maxBytes) { await reader.cancel(); - throw new Error(`Response body exceeded ${maxBytes} bytes`); + assertWithinLimit(totalBytes, maxBytes); } chunks.push(value); } @@ -45,5 +64,9 @@ export async function readResponseWithLimit(response: Response, maxBytes: number offset += chunk.byteLength; } - return new TextDecoder().decode(body); + return body; +} + +export async function readResponseWithLimit(response: Response, maxBytes: number): Promise { + return decoder.decode(await readResponseBytesWithLimit(response, maxBytes)); } diff --git a/tests/unit/skill-github-import-materialize.test.ts b/tests/unit/skill-github-import-materialize.test.ts new file mode 100644 index 00000000..d5e1180f --- /dev/null +++ b/tests/unit/skill-github-import-materialize.test.ts @@ -0,0 +1,92 @@ +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { statSync } from "node:fs"; +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 { createGitHubImporter } from "@main/skills/import-github"; + +const commitSha = "a".repeat(40); + +describe("GitHub skill import materialization", () => { + let tempDir: string; + let db: AppDatabase; + let companyId: string; + + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), "agentcompany-skill-github-import-")); + db = new AppDatabase(tempDir); + db.init(); + companyId = db.core.company.saveCompany({ + name: "GitHub Skill Import Test", + description: "Materializes complete GitHub skill packages", + status: "active", + }); + }); + + afterEach(async () => { + vi.unstubAllGlobals(); + db.close(); + await rm(tempDir, { recursive: true, force: true }); + }); + + it("downloads and materializes GitHub inventory files", async () => { + const fetchMock = vi.fn(async (input: string | URL | Request) => { + const url = String(input); + if (url === "https://api.github.com/repos/acme/skills/commits/HEAD") { + return new Response(commitSha); + } + if (url === `https://api.github.com/repos/acme/skills/git/trees/${commitSha}?recursive=1`) { + return new Response(JSON.stringify({ + sha: commitSha, + tree: [ + { path: "review/SKILL.md", type: "blob", mode: "100644" }, + { path: "review/references/api.md", type: "blob", mode: "100644" }, + { path: "review/scripts/run.sh", type: "blob", mode: "100755" }, + ], + })); + } + if (url === `https://raw.githubusercontent.com/acme/skills/${commitSha}/review/SKILL.md`) { + return new Response("---\nname: Review\nslug: review\n---\n# Review\n"); + } + if (url === `https://raw.githubusercontent.com/acme/skills/${commitSha}/review/references/api.md`) { + return new Response("API notes"); + } + if (url === `https://raw.githubusercontent.com/acme/skills/${commitSha}/review/scripts/run.sh`) { + return new Response("echo review\n"); + } + return new Response("missing", { status: 404 }); + }); + vi.stubGlobal("fetch", fetchMock); + + const importer = createGitHubImporter({ + db, + logger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }, + skillFilesDir: (ownerCompanyId, slug) => join(tempDir, "skills", ownerCompanyId, slug), + }); + + const results = await importer.importFromGitHub(companyId, "https://github.com/acme/skills", null, null); + + expect(results).toHaveLength(1); + const materializedDir = join(tempDir, "skills", companyId, "review"); + await expect(readFile(join(materializedDir, "SKILL.md"), "utf-8")).resolves.toContain("# Review"); + await expect(readFile(join(materializedDir, "references", "api.md"), "utf-8")).resolves.toBe("API notes"); + await expect(readFile(join(materializedDir, "scripts", "run.sh"), "utf-8")).resolves.toBe("echo review\n"); + expect(statSync(join(materializedDir, "scripts", "run.sh")).mode & 0o100).toBe(0o100); + expect(statSync(join(materializedDir, "scripts", "run.sh")).mode & 0o011).toBe(0); + + const [skill] = db.core.skills.listCompanySkills(companyId); + expect(skill?.trustLevel).toBe("scripts_executables"); + expect(skill?.fileInventory).toEqual([ + { path: "SKILL.md", kind: "skill" }, + { path: "references/api.md", kind: "reference" }, + { path: "scripts/run.sh", kind: "script" }, + ]); + expect(fetchMock).toHaveBeenCalledWith(`https://raw.githubusercontent.com/acme/skills/${commitSha}/review/references/api.md`); + expect(fetchMock).toHaveBeenCalledWith(`https://raw.githubusercontent.com/acme/skills/${commitSha}/review/scripts/run.sh`); + }); +}); diff --git a/tests/unit/skill-local-import-materialize.test.ts b/tests/unit/skill-local-import-materialize.test.ts new file mode 100644 index 00000000..f608e89b --- /dev/null +++ b/tests/unit/skill-local-import-materialize.test.ts @@ -0,0 +1,98 @@ +import { existsSync, statSync } from "node:fs"; +import { chmod, mkdir, mkdtemp, readFile, rm, truncate, writeFile } 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_FILE_BYTES, MAX_TOTAL_BYTES } from "@main/skill-github"; +import { importFromLocal } from "@main/skills/import-local"; + +describe("local skill import materialization", () => { + let tempDir: string; + let db: AppDatabase; + let companyId: string; + + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), "agentcompany-skill-local-import-")); + db = new AppDatabase(tempDir); + db.init(); + companyId = db.core.company.saveCompany({ + name: "Local Skill Import Test", + description: "Materializes complete local skill packages", + status: "active", + }); + }); + + afterEach(async () => { + db.close(); + await rm(tempDir, { recursive: true, force: true }); + }); + + it("copies local inventory files and records executable sources as scripts", async () => { + const sourceDir = join(tempDir, "source", "review"); + await mkdir(join(sourceDir, "references"), { recursive: true }); + await mkdir(join(sourceDir, "scripts"), { recursive: true }); + await mkdir(join(sourceDir, "assets"), { recursive: true }); + await writeFile(join(sourceDir, "SKILL.md"), "---\nname: Review\nslug: review\n---\n# Review\n", "utf-8"); + await writeFile(join(sourceDir, "references", "api.md"), "API notes", "utf-8"); + await writeFile(join(sourceDir, "scripts", "run.sh"), "echo review\n", "utf-8"); + await chmod(join(sourceDir, "scripts", "run.sh"), 0o755); + await writeFile(join(sourceDir, "assets", "logo.bin"), new Uint8Array([7, 8, 9])); + + const results = await importFromLocal({ + db, + logger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }, + skillFilesDir: (ownerCompanyId, slug) => join(tempDir, "skills", ownerCompanyId, slug), + }, companyId, sourceDir); + + expect(results).toHaveLength(1); + const materializedDir = join(tempDir, "skills", companyId, "review"); + await expect(readFile(join(materializedDir, "SKILL.md"), "utf-8")).resolves.toContain("# Review"); + await expect(readFile(join(materializedDir, "references", "api.md"), "utf-8")).resolves.toBe("API notes"); + await expect(readFile(join(materializedDir, "scripts", "run.sh"), "utf-8")).resolves.toBe("echo review\n"); + await expect(readFile(join(materializedDir, "assets", "logo.bin"))).resolves.toEqual(Buffer.from([7, 8, 9])); + expect(statSync(join(materializedDir, "scripts", "run.sh")).mode & 0o100).toBe(0o100); + expect(statSync(join(materializedDir, "scripts", "run.sh")).mode & 0o011).toBe(0); + + const [skill] = db.core.skills.listCompanySkills(companyId); + expect(skill?.trustLevel).toBe("scripts_executables"); + expect(skill?.fileInventory).toEqual([ + { path: "SKILL.md", kind: "skill" }, + { path: "assets/logo.bin", kind: "asset" }, + { path: "references/api.md", kind: "reference" }, + { path: "scripts/run.sh", kind: "script" }, + ]); + expect(existsSync(materializedDir)).toBe(true); + }); + + it("rejects local packages whose aggregate size exceeds the limit before reading oversized inventory", async () => { + const sourceDir = join(tempDir, "source", "large"); + await mkdir(join(sourceDir, "assets"), { recursive: true }); + await writeFile(join(sourceDir, "SKILL.md"), "---\nname: Large\nslug: large\n---\n", "utf-8"); + + const fileCount = Math.ceil(MAX_TOTAL_BYTES / MAX_SKILL_FILE_BYTES) + 1; + for (let index = 0; index < fileCount; index += 1) { + await writeFile(join(sourceDir, "assets", `chunk-${index}.bin`), ""); + await truncate(join(sourceDir, "assets", `chunk-${index}.bin`), MAX_SKILL_FILE_BYTES); + } + + await expect(importFromLocal({ + db, + logger: { + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }, + skillFilesDir: (ownerCompanyId, slug) => join(tempDir, "skills", ownerCompanyId, slug), + }, companyId, sourceDir)).rejects.toThrow( + `Skill package exceeded aggregate limit of ${MAX_TOTAL_BYTES} bytes`, + ); + + expect(existsSync(join(tempDir, "skills", companyId, "large"))).toBe(false); + expect(db.core.skills.listCompanySkills(companyId)).toEqual([]); + }); +}); diff --git a/tests/unit/skill-materialize.test.ts b/tests/unit/skill-materialize.test.ts new file mode 100644 index 00000000..1b08937b --- /dev/null +++ b/tests/unit/skill-materialize.test.ts @@ -0,0 +1,84 @@ +import { existsSync, statSync } from "node:fs"; +import { mkdtemp, readFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { MAX_SKILL_FILE_BYTES, MAX_TOTAL_BYTES } from "@main/skill-github"; +import { materializeSkillFiles } from "@main/skills/materialize"; + +describe("skill materialization", () => { + let tempDir: string; + + beforeEach(async () => { + tempDir = await mkdtemp(join(tmpdir(), "agentcompany-skill-materialize-")); + }); + + afterEach(async () => { + await rm(tempDir, { recursive: true, force: true }); + }); + + it("materializes every inventory file and preserves script executability", async () => { + const dir = join(tempDir, "skills", "company", "review"); + + await materializeSkillFiles(dir, [ + { path: "SKILL.md", kind: "skill", content: "---\nname: Review\n---\n" }, + { path: "references/api.md", kind: "reference", content: "API notes" }, + { path: "assets/logo.bin", kind: "asset", content: new Uint8Array([1, 2, 3]) }, + { path: "scripts/run.sh", kind: "script", content: "echo review\n" }, + ]); + + await expect(readFile(join(dir, "SKILL.md"), "utf-8")).resolves.toContain("Review"); + await expect(readFile(join(dir, "references", "api.md"), "utf-8")).resolves.toBe("API notes"); + await expect(readFile(join(dir, "assets", "logo.bin"))).resolves.toEqual(Buffer.from([1, 2, 3])); + await expect(readFile(join(dir, "scripts", "run.sh"), "utf-8")).resolves.toBe("echo review\n"); + expect(statSync(join(dir, "scripts", "run.sh")).mode & 0o100).toBe(0o100); + expect(statSync(join(dir, "scripts", "run.sh")).mode & 0o011).toBe(0); + expect(statSync(join(dir, "references", "api.md")).mode & 0o111).toBe(0); + }); + + it("replaces stale files when a skill is re-materialized", async () => { + const dir = join(tempDir, "skills", "company", "review"); + await materializeSkillFiles(dir, [ + { path: "SKILL.md", kind: "skill", content: "old" }, + { path: "references/stale.md", kind: "reference", content: "stale" }, + ]); + + await materializeSkillFiles(dir, [ + { path: "SKILL.md", kind: "skill", content: "new" }, + ]); + + await expect(readFile(join(dir, "SKILL.md"), "utf-8")).resolves.toBe("new"); + expect(existsSync(join(dir, "references", "stale.md"))).toBe(false); + }); + + it("rejects traversal paths before writing the package", async () => { + const dir = join(tempDir, "skills", "company", "bad"); + + await expect(materializeSkillFiles(dir, [ + { path: "SKILL.md", kind: "skill", content: "ok" }, + { path: "../escape.md", kind: "markdown", content: "bad" }, + ])).rejects.toThrow("Skill file path contains traversal segments: ../escape.md"); + expect(existsSync(dir)).toBe(false); + }); + + it("enforces single-file and aggregate package byte limits", async () => { + const dir = join(tempDir, "skills", "company", "large"); + + await expect(materializeSkillFiles(dir, [ + { path: "SKILL.md", kind: "skill", content: "ok" }, + { path: "assets/too-large.bin", kind: "asset", content: new Uint8Array(MAX_SKILL_FILE_BYTES + 1) }, + ])).rejects.toThrow(`assets/too-large.bin exceeds ${MAX_SKILL_FILE_BYTES}-byte limit`); + + const packageFiles = [ + { path: "SKILL.md", kind: "skill" as const, content: "ok" }, + ...Array.from({ length: Math.ceil(MAX_TOTAL_BYTES / MAX_SKILL_FILE_BYTES) }, (_, index) => ({ + path: `assets/chunk-${index}.bin`, + kind: "asset" as const, + content: new Uint8Array(MAX_SKILL_FILE_BYTES), + })), + ]; + await expect(materializeSkillFiles(dir, packageFiles)).rejects.toThrow( + `Skill package exceeded aggregate limit of ${MAX_TOTAL_BYTES} bytes`, + ); + }); +});