diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fd491e51..c61478140 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -226,6 +226,17 @@ key_refused`; against an older backend the previous behaviour stands. ### Skills +- **Skill Hub installs no longer fail with "ADM-ZIP: No descriptor present"** — + the hub re-packs every uploaded skill to add its metadata file, and that + re-pack keeps each entry's "sizes follow the data" flag while never writing + the trailing record the flag promises. The ZIP reader the desktop app, the + TUI, and `codev skill pull` share refused exactly that shape, so a skill + uploaded from macOS Archive Utility, Windows Explorer, or most streaming + zippers (`outlook-local` was the reported one) could be browsed but never + installed. The reader now validates such entries against the sizes and + checksum the archive's central directory records, which is what every + other unzip tool does; genuinely corrupt data is still refused. + - **Skill Stacks in the desktop app** — the Skill Hub now has two tabs, Skills and Skill Stacks. The Stacks tab lists the hub's stacks with their owner, skill and member counts and a preview of what's inside; Install diff --git a/bun.lock b/bun.lock index 0125c7a07..004e7464d 100644 --- a/bun.lock +++ b/bun.lock @@ -184,7 +184,7 @@ "version": "0.0.1", "dependencies": { "@codev/gateway": "workspace:*", - "adm-zip": "0.5.16", + "adm-zip": "0.6.1", }, "devDependencies": { "@tsconfig/bun": "catalog:", @@ -2989,7 +2989,7 @@ "acorn-walk": ["acorn-walk@8.3.2", "", {}, "sha512-cjkyv4OtNCIeqhHrfS81QWXoCBPExR/J62oyEqepVw8WaQeSqpW2uhuLPh1m9eWhDuOo/jUXVTlifvesOWp/4A=="], - "adm-zip": ["adm-zip@0.5.16", "", {}, "sha512-TGw5yVi4saajsSEgz25grObGHEUaDrniwvA2qwSC060KfqGPdglhvPMA2lPIoxs3PQIItj2iag35fONcQqgUaQ=="], + "adm-zip": ["adm-zip@0.6.1", "", {}, "sha512-Xwrja8nx9e5o2N1my4DsKCeKpdrnACyr1wtbPxBDgGzKzKyE9kRtBFA8mWldI+RVlD7CBZNWY/wQ2+ydwOR6kQ=="], "agent-base": ["agent-base@7.1.4", "", {}, "sha512-MnA+YT8fwfJPgBx3m60MNqakm30XOkyIoH1y6huTQvC0PwZG7ki8NacLBcrPbNoo8vEZy7Jpuk7+jMO+CUovTQ=="], diff --git a/packages/codev-skillhub/package.json b/packages/codev-skillhub/package.json index 011b12629..a966c5808 100644 --- a/packages/codev-skillhub/package.json +++ b/packages/codev-skillhub/package.json @@ -10,7 +10,7 @@ }, "dependencies": { "@codev/gateway": "workspace:*", - "adm-zip": "0.5.16" + "adm-zip": "0.6.1" }, "devDependencies": { "@tsconfig/bun": "catalog:", diff --git a/packages/codev-skillhub/src/archive.ts b/packages/codev-skillhub/src/archive.ts index ef20cdf87..04cb37ca5 100644 --- a/packages/codev-skillhub/src/archive.ts +++ b/packages/codev-skillhub/src/archive.ts @@ -46,10 +46,10 @@ function isIgnoredFile(name: string): boolean { // is absolute or resolves outside the target root is rejected rather than // written. Directory entries are created; file entries are written verbatim. // Entry-count and cumulative-size caps bound a hostile archive (e.g. one pulled -// from the hub) so it can't fill the disk. Note: adm-zip's getData() inflates a -// single entry fully into memory, so a crafted per-entry decompression bomb is -// still bounded only by the compressed input size (see downloadSkill's cap) — -// the cumulative check below is a backstop against many/large honest entries. +// from the hub) so it can't fill the disk. adm-zip (0.6+) inflates each entry +// fully into memory but stops at the size its central header declares, and +// inspectZip has already summed those declared sizes against the cap — the +// cumulative check below is a backstop against many/large honest entries. export async function extractZip(buffer: Buffer, targetDir: string, limits: ArchiveLimits = SKILL_LIMITS) { return extractEntries(new AdmZip(buffer).getEntries(), targetDir, { limits }) } diff --git a/packages/codev-skillhub/test/archive.test.ts b/packages/codev-skillhub/test/archive.test.ts index 14a988f52..ea265d5c0 100644 --- a/packages/codev-skillhub/test/archive.test.ts +++ b/packages/codev-skillhub/test/archive.test.ts @@ -103,3 +103,57 @@ describe("detectAndStripRoot", () => { expect(stripped).toBeNull() }) }) + +// The hub re-serializes every upload with adm-zip to inject _meta.json, and +// adm-zip's writer keeps each original entry's general-purpose flags while +// never emitting a data descriptor. An upload whose zipper set bit 3 (macOS +// Archive Utility, Windows Explorer, most streaming zippers) therefore comes +// back down with the descriptor flag set, real sizes and crc in both headers, +// and no descriptor after the data. adm-zip 0.5.x refused exactly that shape +// ("No descriptor present"); this rewrites a normal archive into it. +function setDescriptorFlag(buffer: Buffer): Buffer { + const out = Buffer.from(buffer) + const LOCSIG = 0x04034b50 + const CENSIG = 0x02014b50 + for (let i = 0; i + 4 <= out.length; i++) { + const sig = out.readUInt32LE(i) + // general-purpose flags sit at +6 in a local header and +8 in a central one + if (sig === LOCSIG) out.writeUInt16LE(out.readUInt16LE(i + 6) | 0x08, i + 6) + if (sig === CENSIG) out.writeUInt16LE(out.readUInt16LE(i + 8) | 0x08, i + 8) + } + return out +} + +describe("an archive with the data-descriptor flag but no descriptor", () => { + const files = { "outlook-local/SKILL.md": "# outlook", "outlook-local/scripts/send.ps1": "Send-Mail" } + + test("marks every entry as flagged", () => { + const zip = setDescriptorFlag(makeZip(files)) + const entries = new AdmZip(zip).getEntries() + expect(entries.every((e) => (e.header.flags & 0x08) !== 0)).toBe(true) + }) + + test("inspects, strips its root, and extracts", async () => { + const zip = setDescriptorFlag(makeZip(files)) + expect(inspectZip(zip).files.sort()).toEqual(["outlook-local/SKILL.md", "outlook-local/scripts/send.ps1"]) + + const { buffer, stripped } = detectAndStripRoot(zip) + expect(stripped).toBe("outlook-local") + expect(entryNames(buffer)).toEqual(["SKILL.md", "scripts/send.ps1"]) + + await extractZip(buffer, tempDir) + expect(readFileSync(join(tempDir, "SKILL.md"), "utf-8")).toBe("# outlook") + expect(readFileSync(join(tempDir, "scripts", "send.ps1"), "utf-8")).toBe("Send-Mail") + }) + + test("still rejects an entry whose bytes do not match the recorded crc", async () => { + const zip = setDescriptorFlag(makeZip({ "SKILL.md": "# outlook" })) + const entry = new AdmZip(zip).getEntry("SKILL.md")! + // Corrupt the stored crc in the central header; the data is unchanged. + const CENSIG = 0x02014b50 + for (let i = 0; i + 4 <= zip.length; i++) { + if (zip.readUInt32LE(i) === CENSIG) zip.writeUInt32LE(entry.header.crc ^ 0xffffffff, i + 16) + } + await expect(extractZip(zip, tempDir)).rejects.toThrow(/CRC/i) + }) +})