Conversation
|
Thanks for the contribution! We're holding off on merging this PR because the newly added Both advisories cover version 2.0.1 and currently list no patched version. We'll wait for an upstream fix, then revisit merging this PR with the fixed dependency. |
|
Completely fair call. Thanks for auditing the dep chain, those advisories are real. One detail worth noting for context. The vulnerable extract-zip path already runs today via Electron's own internals on non-macOS installs, so the exposure predates this PR, declaring the dependency just makes it visible. I'll watch for an upstream patch and update here when one lands. Meanwhile, if you'd prefer, I can rework the script to avoid extract-zip entirely e.g., using Node's built-in unzip capabilities or @electron/get's own extraction, so that non-macOS contributors get unblocked without waiting on the advisory. Happy either way. |
What breaks
Cloning the repo on Windows or Linux and running
npm cifails right away, duringpostinstall:`Error: Cannot find module 'extract-zip'
Require stack:
…\errand\scripts\ensure-electron.mjs`
The install script uses two packages that aren't declared in
package.json:extract-zip— used to arrive as part of Electron itself, but Electron 44 stopped shipping it internally, so on a fresh install it's simply not there anymore.@electron/get— still works today, but only because npm happens to hoist it out of Electron's own dependencies. One resolver change and it breaks the same way.There's also one test that fails on Windows:
appBranding.test.tsexpects forward-slash paths, butstorageDirectorybuilds paths withpath.join, which produces backslashes on Windows.Why you've never seen it
On macOS the script extracts Electron with
ditto, so theextract-zipline never runs and macOS paths use forward slashes anyway. Since CI runs onmacos-15only, both problems are invisible there. They only show up for someone cloning on Windows or Linux which is how I found them.The fix
devDependencies(@electron/get@^5.1.0, which dedupes with Electron's own copy, andextract-zip@^2.0.1). Lockfile updated.appBranding.test.tswithjoin(...)/resolve(...)instead of hardcoded slashes, same pattern as the test's own third assertion already uses. What the test checks hasn't changed.Verified on Windows 11 (Node 24, npm 11)
node_modules, rannpm cifrom scratch installs cleanly, Electron extracts.npm run typecheck,npm run lint,npm test(179/179), andnpm run buildall pass.One small thing I noticed
npm run test:uipoints at a vitest project (--project renderer) that isn't defined invitest.config.ts, so it fails on every platform. Hence I'm happy to fix in a follow-up if you'd like.Thanks for open-sourcing Errand, the codebase was easy to find my way around, and it runs nicely on Windows in dev mode.