Repository navigation
chore(deps): audit the three dependencies that gate provenance and retrieval - #122
Merged
Merged
Conversation
…trieval Closes #62. Produces `docs/dependency-audit.md` covering exactly the three dependencies the issue names — `sqlite-vec`, `pdfjs-dist`, and `@huggingface/transformers` + `onnxruntime-node` — with a verdict and evidence each. Everything else in `package.json` stays out of the document on purpose: an inventory of ~70 direct dependencies generates backlog rather than decisions, so a fourth entry is added only when a committed feature is blocked by it. The document has to open with one piece of context, because the usual convention is inverted in this repo and it is the likeliest way someone mis-fixes a dependency later: `electron.vite.config.ts` externalizes **every** entry in `dependencies` automatically, so `dependencies` is what resolves at runtime from inside the package and `devDependencies` is what rollup **inlines**. "A shipped file imports a devDependency" is therefore correct here, and promoting such an import into `dependencies` would externalize it — which is how v1.2.1 shipped a `MODULE_NOT_FOUND`. Verdicts: - `sqlite-vec` — **KEEP**. `vec0` is the retrieval substrate and its lifecycle behaviour is already asserted end to end by the packaged smoke test. Recorded follow-up: the pin is `0.1.7-alpha.2` while a stable `0.1.9` exists upstream; since the pin is exact that is a deliberate one-line bump plus a smoke run, not something to fold into this change. - `pdfjs-dist` — **REPLACE (upgrade)** → its own issue **#121**, a v1.4 blocker: the resolved `5.7.284` is inside the affected range of GHSA-hq66-cqwq-w95j / CVE-2026-16633. The exploit path is not currently reachable (no `AnnotationLayer`, no `pdfjs-dist/web/*` import, and `PDFScriptingManager` is absent from the API build entirely) but that reasoning rests on our own call sites, and untrusted documents are this app's normal input. - `@huggingface/transformers` + `onnxruntime-node` — **KEEP**. Both are already the latest published versions. Recorded as evidence rather than a task: a linux-x64 build ships 152.7 MB of onnxruntime binaries of which only 43.7 MB is usable, the other ~109 MB being darwin/arm64 (twice, the same binary under two names) and linux/arm64. That is the material fact behind the 747 MB unpacked size, and it is a packaging follow-up on its own merits, not a reason to replace the dependency. FTS5 (which #77 needs before BM25/hybrid is even an option) is answered with a runnable check rather than a claim. It has to be, because `better-sqlite3` is rebuilt for the Electron ABI and cannot be loaded from plain Node at all. The check now lives in the packaged smoke test and inserts, matches and ranks instead of only reading a compile flag — from the shipped build: ``` [SmokeTest] ok FTS5 is available and ranks: sqlite 3.53.2, ENABLE_FTS5 flag present, bm25() = -9.447852760736198e-7 [SmokeTest] PASS - 20 checks passed ``` Also fixed here, because it is a manifest correctness bug rather than an architecture decision: `@ai-sdk/provider` was imported by `src/main/models/protocols/types.ts` as `import type` but declared in neither section, resolving only through hoisting from the five `@ai-sdk/*` / `ai` packages that depend on it. It is now a `devDependency` — dev, not production, both because the import is type-only and because putting it in `dependencies` would externalize it and ship it. The lockfile gains exactly one line and no resolved version moves. Note on "no code change is required by this issue": that holds for the audit itself, but the FTS5 acceptance item asks for a runnable check against the shipped build, which is the smoke-test addition above. Verification: `npm test` (254 pass), `npm run typecheck`, `npm run check:design`, `npm run lint` (0 errors, warning baseline unchanged), `npx prettier --check` on every touched file, and `npm run build:unpack && npm run smoke:packaged` actually executed — 20/20 checks, with the FTS5 result quoted above. `npm ls @ai-sdk/provider` confirms it stays deduped at 2.0.5, and `npm install --package-lock-only --dry-run` reports the lockfile in sync.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #62.
Produces
docs/dependency-audit.mdcovering exactly the three dependencies the issue names, each with a verdict and the evidence behind it. Everything else inpackage.jsonis deliberately left undocumented, per the issue's own rule that bucketing all ~70 direct dependencies generates backlog rather than decisions.sqlite-vecpdfjs-dist@huggingface/transformers+onnxruntime-nodeThe context the document has to open with
electron.vite.config.tsexternalizes every entry independenciesautomatically, so this repo's convention is inverted from the usual one:dependenciesis what resolves at runtime from inside the package, anddevDependenciesis what rollup inlines. "A shipped main-process file imports a devDependency" is therefore correct here — promoting such an import intodependencieswould externalize it, which is a bigger package and a new runtime resolution risk, and is how v1.2.1 shipped aMODULE_NOT_FOUND.Without that paragraph the verdicts below are easy to misread, and it is the single likeliest way someone "fixes" a dependency wrongly later — so it earns a place even though it is not one of the three.
sqlite-vec— KEEPvec0is the retrieval substrate, and the workspace already depends on its lifecycle semantics rather than just its search: per-notebook vector width, a refused width change without an explicit rebuild, and a dropped vector table when a notebook is deleted. All of that is asserted by the packaged smoke test today, and the runtime confirms the shipped version (vec_version()→v0.1.7-alpha.2).Replacing it has no cheap path — a pure-JS index loses the SQLite-side filtering and the shared transaction boundary with
better-sqlite3; another native ANN library means a second native addon inasarUnpack, a new persistence format, and a re-index migration for existing users.Recorded follow-up, not done here: the pin is
0.1.7-alpha.2while a stable0.1.9exists upstream. The pin is exact, so leaving the alpha channel is a one-line manifest change plus a smoke run — worth its own small change so avec0behaviour change cannot hide inside a larger PR.pdfjs-dist— its own issue, #121The resolved
5.7.284is inside the affected range of GHSA-hq66-cqwq-w95j / CVE-2026-16633 (>=5.6.83 <6.2.108, fixed6.2.108). Per this audit's rules, a "replace" recommendation becomes its own issue with its own rollback plan, so it is #121 and marked a v1.4 blocker — not a dependency bump hidden inside an audit PR.For the record, the reachability conclusion recorded in #121 is that the exploit path is not currently reachable: neither PDF path passes
enableScriptingand neither builds anAnnotationLayer,PDFScriptingManageris absent from the API build (barepdfjs-distresolves tomain: build/pdf.mjs), and the only other import is the parsing worker. It is still a blocker, because that conclusion rests on our call sites, and untrusted PDFs are this app's normal input.@huggingface/transformers+onnxruntime-node— KEEPBoth are already the latest published versions (4.3.0, 1.30.0), and this pair is the local embedding backend, so replacing it would break the local-first premise. The model is pinned by revision with file sizes and SHA256s verified before anything is cached; the first-use download is 134 MB.
Recorded as evidence, not a task: a linux-x64 build ships 152.7 MB of onnxruntime binaries, of which only
linux/x64(43.7 MB) is usable.linux/x64/libonnxruntime.so.1darwin/arm64/libonnxruntime.1.dylibdarwin/arm64/libonnxruntime.1.30.0.dyliblinux/arm64/libonnxruntime.so.1That is the material fact behind the 747 MB unpacked figure, and it is more accurate as "the embedding runtime ships the wrong platforms" than as "the embedding runtime is large". Pruning it is a packaging change with its own risk (a wrong filter produces an app that starts on the CI runner and crashes on a user's machine), so it is not part of this PR and does not change the verdict on the dependency.
FTS5 — answered with a runnable check
#77 needs this before BM25/hybrid is even an option, and it could not be answered with a document claim:
better-sqlite3is rebuilt for the Electron ABI and cannot be loaded from plain Node at all, so it has to be asked inside Electron. The check now lives in the packaged smoke test and inserts, matches and ranks — a compile flag is not proof thatMATCHandbm25()work in the build users run.Available. #77 has no blocker from the SQLite side; BM25/hybrid is a design choice rather than a build constraint.
Also fixed: an undeclared type import
@ai-sdk/provideris imported bysrc/main/models/protocols/types.tsasimport type, but declared in neither section — it resolved only by hoisting from the five@ai-sdk/*/aipackages that depend on it. Now adevDependency: dev rather than production both because the import is type-only and becausedependencieswould externalize and ship it.The lockfile gains exactly one line, no resolved version moves,
npm ls @ai-sdk/providerstays deduped at 2.0.5, andnpm install --package-lock-only --dry-runreports the lock in sync.Acceptance
docs/dependency-audit.mdcovers the three, with a recommendation eachsqlite-vec's FTS5 availability answered with evidence, by a runnable check against the shipped buildVerification
npm run build:unpack && npm run smoke:packaged— 20/20 checks, run on the packaged app, FTS5 result quoted abovenpm test— 254 pass / 0 failnpm run typecheck,npm run check:designnpm run lint— 0 errors, warning baseline unchanged at 107npx prettier --checkon every touched filenpm run check:versionstill greenNot in this PR
sqlite-vecalpha → stable bump — recorded as a follow-up in the document.docs/evalprettier churn (the committed baseline is not prettier-clean, so anyprettier --writeover the repo reformats it) — its own change, by request.