From 120deebf7137859c198669e81979ed3972bcc391 Mon Sep 17 00:00:00 2001 From: minhn4 Date: Wed, 16 Sep 2026 16:48:10 +0700 Subject: [PATCH] fix(skillhub): accept hub archives that set the data-descriptor flag Installing some skills from the Skill Hub failed with "ADM-ZIP: No descriptor present". The hub re-packs every upload with adm-zip to inject _meta.json; adm-zip's writer keeps each entry's original general-purpose flags but never emits a data descriptor, so an upload whose zipper set bit 3 (macOS Archive Utility, Windows Explorer, streaming zippers) comes back with the flag set, real sizes/crc in both headers, and no descriptor. adm-zip 0.5.16 refused exactly that shape; 0.6.1 validates such entries against the central directory's crc instead (upstream issues #533/#548/#554), which is what every other unzip tool does. Bump adm-zip to 0.6.1 and add a regression test that rewrites a normal archive into the hub's shape. 0.6.1 also caps inflation at each entry's declared size and rejects duplicate entry names, both of which tighten the guards archive.ts already relies on. Co-Authored-By: Claude Fable 5.1 --- CHANGELOG.md | 11 ++++ bun.lock | 4 +- packages/codev-skillhub/package.json | 2 +- packages/codev-skillhub/src/archive.ts | 8 +-- packages/codev-skillhub/test/archive.test.ts | 54 ++++++++++++++++++++ 5 files changed, 72 insertions(+), 7 deletions(-) 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) + }) +})