From 2a64a34ca92059cabf0b14e80e606d99d6b427f3 Mon Sep 17 00:00:00 2001 From: Quickbeard Date: Wed, 5 Aug 2026 11:23:22 +0700 Subject: [PATCH 1/2] Never splice a resumed download across a republished bundle A user's `codevhub skill office` run produced a corrupt bundle zip ("bad zipfile offset" a few KB in): a leftover .partial from the old publish was resumed with Range and the new object's bytes were appended onto stale ones. Two holes closed in downloadFile: - A partial with no stored ETag (pre-ETag download or cleared record) used to resume with a bare Range and trust the 206. Without an expected sha256 nothing downstream catches the splice, so such a partial is now scrapped and the download starts clean. - Even with If-Range, a hop that mishandles it can answer 206 for a republished entity. A 206 whose ETag differs from the partial's is now treated like the 416 case: scrap the partial and ETag record and refetch from scratch. Test server gains a brokenIfRange mode (honors Range while ignoring If-Range) to cover the second case. Co-Authored-By: Claude Fable 5 --- src/lib/download.ts | 32 ++++++++++++++++++++++++++----- tests/lib/download.test.ts | 39 +++++++++++++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/src/lib/download.ts b/src/lib/download.ts index 1678875..fa04cc4 100644 --- a/src/lib/download.ts +++ b/src/lib/download.ts @@ -115,11 +115,20 @@ export async function downloadFile(opts: DownloadOptions): Promise { let hash = createHash("sha256"); let offset = 0; if (existsSync(partial)) { - offset = statSync(partial).size; - const h = hash; - await pipeline(createReadStream(partial), async (chunks) => { - for await (const chunk of chunks) h.update(chunk as Buffer); - }); + // Without an expected sha256 there is nothing downstream to catch a bad + // splice, so a resume is only trustworthy when the partial's ETag is on + // record to send as If-Range. A bare Range answered with 206 would + // append the republished object's bytes onto stale ones — scrap such a + // partial and download from scratch instead. + if (!opts.sha256 && !readEtag(etagFile)) { + rmSync(partial, { force: true }); + } else { + offset = statSync(partial).size; + const h = hash; + await pipeline(createReadStream(partial), async (chunks) => { + for await (const chunk of chunks) h.update(chunk as Buffer); + }); + } } // If-Range makes a resume safe across republishes: when the entity no @@ -138,6 +147,19 @@ export async function downloadFile(opts: DownloadOptions): Promise { let append = false; if (res.status === 206 && offset > 0) { + // Belt and braces on top of If-Range: a middlebox that mishandles it + // can still answer 206 for a republished entity (this corrupted real + // bundle downloads — "bad zipfile offset" from spliced halves). When + // the 206 carries an ETag that differs from the partial's, the range + // is against a different object: scrap the partial and refetch clean. + const storedEtag = readEtag(etagFile); + const gotEtag = res.headers.get("etag"); + if (storedEtag && gotEtag && gotEtag !== storedEtag) { + await res.body?.cancel(); + rmSync(partial, { force: true }); + rmSync(etagFile, { force: true }); + return downloadFile(opts); + } append = true; } else if (res.ok) { // 200 despite Range (server ignored it) or a fresh download: start over. diff --git a/tests/lib/download.test.ts b/tests/lib/download.test.ts index 3f1b5ff..3ae7ba2 100644 --- a/tests/lib/download.test.ts +++ b/tests/lib/download.test.ts @@ -54,6 +54,9 @@ let ignoreRange = false; // When true the server sends no Content-Length (chunked), as a proxy or a // streaming origin may. let omitContentLength = false; +// Simulate a middlebox that honors Range but ignores If-Range semantics — +// answering 206 (with the current entity's ETag) even when If-Range mismatches. +let brokenIfRange = false; // Extra objects (path -> body) the server should serve. let objects: Map; @@ -62,6 +65,7 @@ beforeEach(async () => { rangeLog = []; ignoreRange = false; omitContentLength = false; + brokenIfRange = false; objects = new Map([["/payload.bin", PAYLOAD]]); server = createServer((req, res) => { const body = objects.get(req.url ?? ""); @@ -79,7 +83,11 @@ beforeEach(async () => { return; } const ifRange = req.headers["if-range"]; - if (range && !ignoreRange && (ifRange === undefined || ifRange === etag)) { + if ( + range && + !ignoreRange && + (brokenIfRange || ifRange === undefined || ifRange === etag) + ) { const start = Number(/^bytes=(\d+)-$/.exec(range)?.[1]); if (!Number.isFinite(start) || start >= body.length) { res.writeHead(416).end(); @@ -279,6 +287,35 @@ describe("downloadFile ETag revalidation", () => { // If-Range mismatched → server sent 200 → clean restart, correct bytes. expect(readFileSync(dest).equals(PAYLOAD)).toBe(true); }); + + test("refuses a bare-Range resume of an ETag-less partial without a sha256", async () => { + const dest = join(tempDir, "payload.bin"); + // A partial with no `.etag` on record (pre-ETag download, or a cleared + // record). Without an expected sha256 nothing downstream would catch a + // splice, so the partial must be scrapped, not resumed. + writeFileSync(`${dest}.partial`, Buffer.from("stale-old-bytes")); + await downloadFile({ url: `${baseUrl}/payload.bin`, dest, endpoint: "t" }); + // No Range header ever reached the server — a clean full download. + expect(rangeLog).toEqual([undefined]); + expect(readFileSync(dest).equals(PAYLOAD)).toBe(true); + expect(existsSync(`${dest}.partial`)).toBe(false); + }); + + test("scraps the partial when a broken hop answers 206 across a republish", async () => { + // A middlebox that honors Range while ignoring If-Range: it answers 206 + // for a republished entity, which used to splice the halves together + // ("bad zipfile offset" on real bundle downloads). + brokenIfRange = true; + const dest = join(tempDir, "payload.bin"); + writeFileSync(`${dest}.partial`, Buffer.from("stale-old-bytes")); + writeFileSync(`${dest}.etag`, '"stale-etag"'); + await downloadFile({ url: `${baseUrl}/payload.bin`, dest, endpoint: "t" }); + // The lying 206 carried the new object's ETag → partial scrapped, then + // a clean full refetch (no Range on the second request). + expect(rangeLog).toEqual(["bytes=15-", undefined]); + expect(readFileSync(dest).equals(PAYLOAD)).toBe(true); + expect(existsSync(`${dest}.partial`)).toBe(false); + }); }); // End-to-end through runSkillOffice against the local server. The test host is From af70bb439ccf946224b2522fc891393bcd704c96 Mon Sep 17 00:00:00 2001 From: Quickbeard Date: Wed, 5 Aug 2026 11:28:34 +0700 Subject: [PATCH 2/2] bump version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index bf1b93c..897f530 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "codev-ai", - "version": "0.5.9", + "version": "0.5.10", "description": "CoDev — AI Coding Agent Hub. Install, configure, and manage multiple AI coding agents.", "keywords": [ "ai",