From ac1bce73e56685d56160895c8141e1f2a4507237 Mon Sep 17 00:00:00 2001 From: Arham Wani Date: Tue, 4 Aug 2026 04:47:51 +0530 Subject: [PATCH 1/2] fix(stt): verify cached model checksum --- electron/stt/modelManager.test.ts | 51 +++++++++++++++++++++++++------ electron/stt/modelManager.ts | 22 ++++++++----- 2 files changed, 56 insertions(+), 17 deletions(-) diff --git a/electron/stt/modelManager.test.ts b/electron/stt/modelManager.test.ts index 92145dc7a..fa9b93a3d 100644 --- a/electron/stt/modelManager.test.ts +++ b/electron/stt/modelManager.test.ts @@ -1,4 +1,5 @@ -import { mkdir, mkdtemp, rm, stat, writeFile } from "node:fs/promises"; +import { createHash } from "node:crypto"; +import { mkdir, mkdtemp, readFile, rm, stat, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; @@ -45,19 +46,51 @@ describe("modelManager", () => { it("ensureModels succeeds when the file is already present (cache hit)", async () => { const paths = modelPaths(dir); await mkdir(path.dirname(paths.whisper), { recursive: true }); - await writeFile(paths.whisper, "dummy-ggml"); + const cached = Buffer.from("dummy-ggml"); + await writeFile(paths.whisper, cached); + const originalSha = STT_MODELS.whisper.files[0].expectedSha256; + STT_MODELS.whisper.files[0].expectedSha256 = createHash("sha256").update(cached).digest("hex"); let fetches = 0; const fetcher: typeof fetch = async () => { fetches++; return new Response("should not be reached", { status: 200 }); }; - await ensureModels({ - baseDir: dir, - only: ["whisper"], - fetcher, - onProgress: () => undefined, - }); - expect(fetches).toBe(0); + try { + await ensureModels({ + baseDir: dir, + only: ["whisper"], + fetcher, + onProgress: () => undefined, + }); + expect(fetches).toBe(0); + } finally { + STT_MODELS.whisper.files[0].expectedSha256 = originalSha; + } + }); + + it("re-downloads a non-empty cached model when its checksum is wrong", async () => { + const paths = modelPaths(dir); + await mkdir(path.dirname(paths.whisper), { recursive: true }); + await writeFile(paths.whisper, "corrupt-cache"); + const replacement = Buffer.from("verified-ggml-weights"); + const originalSha = STT_MODELS.whisper.files[0].expectedSha256; + STT_MODELS.whisper.files[0].expectedSha256 = createHash("sha256") + .update(replacement) + .digest("hex"); + let fetches = 0; + const fetcher: typeof fetch = async () => { + fetches++; + return new Response(replacement, { status: 200 }); + }; + + try { + await ensureModels({ baseDir: dir, only: ["whisper"], fetcher }); + expect(fetches).toBe(1); + expect(await readFile(paths.whisper)).toEqual(replacement); + expect(await readFile(`${paths.whisper}.bad`, "utf8")).toBe("corrupt-cache"); + } finally { + STT_MODELS.whisper.files[0].expectedSha256 = originalSha; + } }); it("ensureModels downloads the missing GGML file with progress", async () => { diff --git a/electron/stt/modelManager.ts b/electron/stt/modelManager.ts index 8ec47652e..6265cd786 100644 --- a/electron/stt/modelManager.ts +++ b/electron/stt/modelManager.ts @@ -1,6 +1,6 @@ import { createHash } from "node:crypto"; import { createReadStream, existsSync } from "node:fs"; -import { mkdir, rename, stat } from "node:fs/promises"; +import { mkdir, rename, rm, stat } from "node:fs/promises"; import path from "node:path"; import { Readable } from "node:stream"; import { pipeline } from "node:stream/promises"; @@ -153,7 +153,8 @@ export interface DownloadOptions { * Stream a model file to disk atomically (.partial → rename on * success), optionally verify the SHA-256. * - * If the file already exists and is non-empty, skips the download. + * If the file already exists, is non-empty, and matches the expected hash, + * skips the download. Invalid cache entries are quarantined as `.bad`. */ async function ensureFile( filePath: string, @@ -164,7 +165,12 @@ async function ensureFile( if (existsSync(filePath)) { const s = await stat(filePath); if (s.isFile() && s.size > 0) { - return; + if (!expectedSha256) return; + const actual = await sha256OfFile(filePath); + if (actual.toLowerCase() === expectedSha256.toLowerCase()) return; + const badPath = `${filePath}.bad`; + await rm(badPath, { force: true }); + await rename(filePath, badPath); } } @@ -182,17 +188,19 @@ async function ensureFile( }); const { createWriteStream } = await import("node:fs"); await pipeline(source, createWriteStream(tmp)); - await rename(tmp, filePath); if (expectedSha256) { - const actual = await sha256OfFile(filePath); + const actual = await sha256OfFile(tmp); if (actual.toLowerCase() !== expectedSha256.toLowerCase()) { - await rename(filePath, `${filePath}.bad`).catch(() => undefined); + const badPath = `${filePath}.bad`; + await rm(badPath, { force: true }); + await rename(tmp, badPath); throw new Error( `SHA-256 mismatch for ${path.basename(filePath)}: expected ${expectedSha256}, got ${actual}`, ); } } + await rename(tmp, filePath); } export interface EnsureModelsOptions { @@ -217,8 +225,6 @@ export async function ensureModels(opts: EnsureModelsOptions): Promise { })); for (const { id, descriptor, filePath } of targets) { - if (await areModelsPresent(opts.baseDir)) continue; - await mkdir(path.dirname(filePath), { recursive: true }); const file = descriptor.files[0]; From d81bcbb18c7148a7cb1549f1d5603d85de77281f Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Tue, 4 Aug 2026 14:44:40 +0200 Subject: [PATCH 2/2] fix(stt): verify the cached model without risking the user's only copy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The cache path never checked the digest it carries: ensureFile returned as soon as the file existed and was non-empty, so a model truncated by a full disk or scrambled by a bad sector was handed straight to whisper.cpp, and the only symptom the user got was garbage captions. ensureModels also bailed out on areModelsPresent before touching the loop body, which made the check about "is any model present" rather than "is this model present" — the moment a second entry joins STT_MODELS it would silently never download. Verifying the cache means an existing install can now fail the check on a file it has been happily using, so what happens next matters more than it used to. The stale copy stays exactly where it is while the replacement downloads: the final rename is already atomic, so moving it aside first closes no window, while costing the user their working model if the replacement never lands — offline, HuggingFace 5xx, no space — and stranding 264 MB that nothing ever reaps. If the replacement mismatches too, the download is discarded and the original bytes are kept, because a double mismatch is precisely when those bytes are worth looking at. That cleanup is guarded: a Windows AV scanner still holding the handle raises EPERM, and the bare errno would escape in place of the mismatch message that actually tells the user what went wrong. The URL now resolves through a pinned commit instead of resolve/main. A mutable branch pointer made the recorded digest a bet, and a re-upload used to break only new installs; with the cache verified on every start it would invalidate every installed cache at once. The pinned revision was checked against HuggingFace's paths-info API: its LFS oid for ggml-small-q8_0.bin is the digest already in the file, byte for byte. Bumping the model now means bumping the revision and the digest together. --- electron/stt/modelManager.test.ts | 50 ++++++++++++++++++- electron/stt/modelManager.ts | 28 ++++++++--- .../transcription-and-captions.md | 17 +++++-- 3 files changed, 83 insertions(+), 12 deletions(-) diff --git a/electron/stt/modelManager.test.ts b/electron/stt/modelManager.test.ts index fa9b93a3d..e5f186fbd 100644 --- a/electron/stt/modelManager.test.ts +++ b/electron/stt/modelManager.test.ts @@ -1,4 +1,5 @@ import { createHash } from "node:crypto"; +import { existsSync } from "node:fs"; import { mkdir, mkdtemp, readFile, rm, stat, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; @@ -23,6 +24,9 @@ describe("modelManager", () => { for (const f of STT_MODELS.whisper.files) { expect(f.approximateBytes).toBeGreaterThan(0); expect(f.url).toContain("huggingface.co"); + // Pinned to an immutable commit: resolving through `main` would let a + // re-upload invalidate every cached model in the field at once. + expect(f.url).toMatch(/\/resolve\/[0-9a-f]{40}\//); } }); @@ -87,7 +91,51 @@ describe("modelManager", () => { await ensureModels({ baseDir: dir, only: ["whisper"], fetcher }); expect(fetches).toBe(1); expect(await readFile(paths.whisper)).toEqual(replacement); - expect(await readFile(`${paths.whisper}.bad`, "utf8")).toBe("corrupt-cache"); + // The stale copy is displaced by the atomic rename, not quarantined + // beside it: a `.bad` sibling would strand 264 MB nothing ever reaps. + expect(existsSync(`${paths.whisper}.bad`)).toBe(false); + expect(existsSync(`${paths.whisper}.partial`)).toBe(false); + } finally { + STT_MODELS.whisper.files[0].expectedSha256 = originalSha; + } + }); + + it("never lets a mismatching download occupy the live model path", async () => { + const paths = modelPaths(dir); + const originalSha = STT_MODELS.whisper.files[0].expectedSha256; + STT_MODELS.whisper.files[0].expectedSha256 = createHash("sha256") + .update("the-weights-we-asked-for") + .digest("hex"); + const served = Buffer.from("truncated-or-tampered-weights"); + const fetcher: typeof fetch = async () => new Response(served, { status: 200 }); + + try { + await expect(ensureModels({ baseDir: dir, only: ["whisper"], fetcher })).rejects.toThrow( + /SHA-256 mismatch/, + ); + expect(existsSync(paths.whisper)).toBe(false); + expect(existsSync(`${paths.whisper}.partial`)).toBe(false); + } finally { + STT_MODELS.whisper.files[0].expectedSha256 = originalSha; + } + }); + + it("keeps the cached model when the replacement download also mismatches", async () => { + const paths = modelPaths(dir); + await mkdir(path.dirname(paths.whisper), { recursive: true }); + await writeFile(paths.whisper, "the-only-copy-the-user-has"); + const originalSha = STT_MODELS.whisper.files[0].expectedSha256; + STT_MODELS.whisper.files[0].expectedSha256 = createHash("sha256") + .update("the-weights-we-asked-for") + .digest("hex"); + const fetcher: typeof fetch = async () => + new Response(Buffer.from("also-wrong"), { status: 200 }); + + try { + await expect(ensureModels({ baseDir: dir, only: ["whisper"], fetcher })).rejects.toThrow( + /SHA-256 mismatch/, + ); + expect(await readFile(paths.whisper, "utf8")).toBe("the-only-copy-the-user-has"); } finally { STT_MODELS.whisper.files[0].expectedSha256 = originalSha; } diff --git a/electron/stt/modelManager.ts b/electron/stt/modelManager.ts index 6265cd786..cb13fd31f 100644 --- a/electron/stt/modelManager.ts +++ b/electron/stt/modelManager.ts @@ -55,6 +55,12 @@ const MODEL_BASE = "https://huggingface.co"; // GitHub org was renamed. const MODEL_REPO = "ggerganov/whisper.cpp"; const MODEL_FILE = "ggml-small-q8_0.bin"; +// Pinned to a commit rather than `main` so `expectedSha256` is an invariant and +// not a bet: `main` is a mutable branch pointer, and a re-upload under it would +// now invalidate every cache in the field at once instead of merely breaking new +// installs. This revision was checked against HuggingFace's paths-info API — its +// LFS oid for MODEL_FILE is exactly the digest below. +const MODEL_REVISION = "5359861c739e955e79d9a303bcbc70fb988958b1"; export const STT_MODELS: Record = { whisper: { @@ -63,7 +69,7 @@ export const STT_MODELS: Record = { files: [ { name: MODEL_FILE, - url: `${MODEL_BASE}/${MODEL_REPO}/resolve/main/${MODEL_FILE}`, + url: `${MODEL_BASE}/${MODEL_REPO}/resolve/${MODEL_REVISION}/${MODEL_FILE}`, expectedSha256: "49C8FB02B65E6049D5FA6C04F81F53B867B5EC9540406812C643F177317F779F", approximateBytes: 264_000_000, }, @@ -154,7 +160,8 @@ export interface DownloadOptions { * success), optionally verify the SHA-256. * * If the file already exists, is non-empty, and matches the expected hash, - * skips the download. Invalid cache entries are quarantined as `.bad`. + * skips the download; otherwise a replacement is fetched and the stale copy is + * only displaced once that replacement has itself been verified. */ async function ensureFile( filePath: string, @@ -168,9 +175,11 @@ async function ensureFile( if (!expectedSha256) return; const actual = await sha256OfFile(filePath); if (actual.toLowerCase() === expectedSha256.toLowerCase()) return; - const badPath = `${filePath}.bad`; - await rm(badPath, { force: true }); - await rename(filePath, badPath); + // Deliberately leave the stale file where it is. Moving it aside now + // would buy nothing — the rename at the end of this function is already + // atomic, so there is no window to close — while costing the user their + // only model if the replacement never lands (offline, HF 5xx, ENOSPC) + // and stranding 264 MB that nothing ever cleans up. } } @@ -192,9 +201,12 @@ async function ensureFile( if (expectedSha256) { const actual = await sha256OfFile(tmp); if (actual.toLowerCase() !== expectedSha256.toLowerCase()) { - const badPath = `${filePath}.bad`; - await rm(badPath, { force: true }); - await rename(tmp, badPath); + // Drop the bad download and keep whatever was already on disk: when both + // copies mismatch, the bytes the user has been running are exactly the + // ones worth diagnosing. The cleanup is guarded because a Windows AV + // scanner still holding the handle raises EPERM/EBUSY, and that bare + // errno would escape in place of the mismatch message below. + await rm(tmp, { force: true }).catch(() => undefined); throw new Error( `SHA-256 mismatch for ${path.basename(filePath)}: expected ${expectedSha256}, got ${actual}`, ); diff --git a/technical-documentation/architecture/transcription-and-captions.md b/technical-documentation/architecture/transcription-and-captions.md index cf60e7c6b..47538bc65 100644 --- a/technical-documentation/architecture/transcription-and-captions.md +++ b/technical-documentation/architecture/transcription-and-captions.md @@ -234,9 +234,20 @@ The single shipped artifact is `ggml-small-q8_0.bin` from `ggerganov/whisper.cpp` on HuggingFace: Whisper `small`, multilingual (~99 languages), q8_0 quantised, ~264 MB. Precision is baked into the GGML file — there is no runtime `--int8` flag. `electron/stt/modelManager.ts` downloads -the file once into the user-data cache, verifies its SHA-256, and writes it -through an atomic `.partial` rename, so a half-downloaded file can never be -picked up as a usable model. +the file once into the user-data cache and writes it through an atomic +`.partial` rename, so a half-downloaded file can never be picked up as a +usable model. The SHA-256 is checked on the cached copy too, not only on a +fresh download, so a model corrupted after the fact is re-fetched rather than +handed to whisper.cpp. A cached copy that fails the check is left alone until +a verified replacement has landed — the atomic rename displaces it — so a +failed re-download never leaves a user with no model at all. + +The download URL resolves through an immutable commit revision rather than +`resolve/main`. Because the cache is now verified on every start, a re-upload +under the mutable branch pointer would invalidate every installed cache at +once instead of merely breaking new installs; pinning makes the recorded +digest an invariant. Bumping the model therefore means bumping the revision +and the digest together. > The HuggingFace identifier is intentionally `ggerganov/whisper.cpp`, > **not** `ggml-org/whisper.cpp`. The latter matches the GitHub org the