Upgrade dependencies: 79 advisories down to 24, no criticals - #719
Merged
Merged
Conversation
Electron 41.7.1 -> 41.10.7 clears GHSA-r4w5-6pfg-jxp5 and GHSA-9f4c-93c8-jc8g; better-sqlite3 12.10.0 -> 12.11.1 is the highest 12.x actually published to npm (12.11.2 and 12.12.0 exist as GitHub tags only). npm pins allowScripts to an exact name@version, so both bumps invalidated their entry and the install scripts were blocked. The Electron binary silently disappeared, which breaks every test, since the suite runs inside Electron. Re-approved both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@electron/osx-sign 2.4.0 -> 2.7.0 and @electron/rebuild 4.0.4 -> 4.2.0, both additive releases. @electron-forge/* is already at its latest published version, so there is nothing to take there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vitest, vite, eslint/typescript-eslint, vue and @atproto each move as a unit, plus the independent leaves. Two snags worth recording. npm's incremental resolver deadlocked on the typescript-eslint cluster and only cleared after uninstalling and reinstalling the four packages together. npm outdated reported typescript-eslint's latest as 8.70.0 when the registry had 8.70.1, and @typescript-eslint/eslint-plugin peer-requires a matching parser, so the stale number was itself the deadlock. Prettier 3.9.8 reformats three files. No behaviour changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resolves 16 advisories, against 5 for every direct upgrade combined, because nearly all of them sit in transitive dependencies that a direct bump never reaches. Lockfile only; package.json is untouched. Needs --allow-git=all: @electron-forge/cli pulls @electron/rebuild 3.7.2, which resolves node-gyp from a git URL, and npm now refuses git fetches by default. Never --force. npm offers to "fix" tar, tmp and extract-zip by downgrading @electron-forge/cli from 7.11.2 to 7.6.1, and the Vue CLI advisories by taking @vue/cli-plugin-typescript back to 3.12.1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Clears all three criticals and both remaining lows. npm reports fixAvailable: false for these, which means it cannot reach them by upgrading a direct dependency, not that no patched release exists. tar, tmp and image-size all had safe versions on the registry. mhtml2html is the interesting one. It pins jsdom ^15.1.1, which drags in request, form-data, qs and tough-cookie, and that chain held two of the three criticals. archive.ts passes its own parseDOM built from the root jsdom, so the bundled copy was never doing any work. Overriding it to ^29.1.1 drops the chain; verified by converting an MHTML fixture end to end. An override only re-resolves a subtree npm has not already pinned, so mhtml2html needed an uninstall and reinstall to take effect. uuid is forced to ^11.1.1 for http-mitm-proxy and sockjs. http-mitm-proxy imports only the named v4 export, which v11 keeps. extract-zip is left alone: every published version is vulnerable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
12.11.1 -> 13.0.3. Version 13 is the first release built on node-addon-api, so it publishes portable prebuilds instead of compiling per ABI, and it was the only native module in the tree. electron-rebuild now has nothing to build, and the binary survives an Electron major on its own. Types follow to @types/better-sqlite3 9.6.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
41.10.7 -> 44.4.3, crossing three majors off an end-of-support line. ABI 145 -> 149, Chromium 152, Node 24.21. Cyd is barely exposed to the breaking changes. It does not touch the clipboard, login item settings or subframe workers. clearStorageData is called with no arguments, so losing options.quotas costs nothing, and showHiddenFiles survives as a type union member. Forge builds for the host arch and CI covers only x64 and arm64, so dropping ia32 and armv7l changes nothing here. macOS 12 users are dropped, which is a support decision rather than a code one. better-sqlite3 needed no rebuild across the ABI jump, which is why it moved to 13 first. node-abi is overridden to ^4.35.0: the version @electron/rebuild pins only knows Electron up to 43, and electron-rebuild fails outright on 44 without it. Verified: tests, lint, the renderer vite build, the x-archive build, and electron-forge package. The makers and the scripts/clean.mjs path are untested here, as clean.mjs needs an interactive sudo to chown chrome-sandbox. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Vue CLI stopped at 5.0.9 and will not be patched again, so its webpack toolchain was the last large source of advisories and the only one npm offered to "fix" by downgrading to 3.12.1. Dropping it removes 464 packages and every remaining moderate. The archive is opened by double-clicking index.html, and browsers refuse to fetch an ES module over file://. Vite tags its entry as a module regardless of the output format, so a small plugin rewrites the tag back to a classic deferred script and drops crossorigin from the entry and the stylesheet. defer matters: the tag sits in head, and a bare classic script runs before #app is parsed and mounts onto nothing. Verified by loading the built archive over file:// with seeded archive data, checking the document title, a rendered tweet, Bootstrap layout and Font Awesome fonts. index.html moves to the project root, which empties public/, so build-archive-sites.sh no longer has a stale archive.js to delete. vue-class-component was unused and is dropped. The components imported defineProps from vue, which collides with the compiler macro; Vue CLI never type-checked strictly enough to notice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Compiled from the 79 -> 24 pass, so every claim in it is something that actually bit during that run rather than general npm advice. The load-bearing parts are the ones no amount of reading the manifest would tell you: allowScripts pins install-script approval to an exact name@version, so any bump of Electron or better-sqlite3 silently blocks the script and the test suite loses the binary it runs inside; npm audit fix moves the transitive pins where most of the advisories actually live; and fixAvailable: false means npm cannot reach a fix by bumping a direct dependency, not that no patched release exists. The major-upgrade playbook is a separate file, since only some runs reach it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The majors left two dead pins behind: electron@41.10.7, which no longer runs a script at all since v42 downloads on first run, and better-sqlite3@12.11.1, superseded by 13.0.3. A stale pin is invisible until someone's install blocks, which is what make-local surfaced. better-sqlite3 13 declares no install script and the lockfile sets no hasInstallScript for it, but npm still runs an implicit node-gyp rebuild for anything shipping a binding.gyp, so it keeps an entry. It only matters where no prebuild exists and the source build is the fallback. Adds the reconciliation pass to the skill as its own phase, since approving as you go is what creates the mess. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Forge pins @electron/rebuild 3.7.2, which resolves node-gyp from a git URL. npm from 12 refuses git fetches by default, so a clean install of this repo failed outright with EALLOWGIT. It only appeared to work here because the package was already unpacked in node_modules; a fresh clone on npm 12 could not install at all. @electron/rebuild 4.2.0 takes node-gyp from the registry, so overriding it removes the git dependency rather than reopening git fetching to work around one transitive. It also dedupes the two copies Forge and the root were each carrying, and drops 102 packages. That supersedes the node-abi override from the Electron 44 commit: 4.2.0 depends on node-abi ^4.2.0, which resolves to a version that already knows Electron 44 and 45. Two overrides become one. Verified electron-rebuild still builds better-sqlite3 and Forge still drives it, by making all three Linux distributables. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The release image was Node 22 with npm 10, two lines behind the runtime and a major behind the npm on a developer's machine. The npm gap was not cosmetic. npm 12 records a libc field on optional platform packages that npm 10 does not understand and silently strips, so every release build quietly rewrote package-lock.json and every developer rewrote it back. Matching versions also means the allowScripts allowlist is exercised where releases are built, instead of only failing on a laptop. Bookworm stays. The postinstall rebuilds better-sqlite3 from source against the image's glibc, so the base image sets the floor for every Linux user: bookworm is 2.36, trixie would make it 2.41. Node 24 LTS is available on bookworm, so the runtime moves and the floor does not. Verified by building the deb, rpm and zip in the container. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
inlineDynamicImports is ignored when codeSplitting is off, which cssCodeSplit: false already implies, and Vite says so on every build. Output hash is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Vite 8.3 warns that a config written in ESM but loaded as CommonJS will break when configLoader: 'native' becomes the default. The root tsconfig sets module: CommonJS, so all four configs were in that position. Renaming them to .mts makes them genuinely ESM, which then wants what native ESM always wants: an import attribute on the package.json import, full specifiers on relative imports, and import.meta.dirname in place of __dirname. forge.config.ts stays CommonJS and keeps using __dirname. Also takes the rolldown deprecation that arrived with the same bump: inlineDynamicImports is now spelled codeSplitting: false. Verified by packaging and by starting the app; both build clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
archive.js is written into the archive folder by Cyd, long after this build runs, so a tag for it in index.html points at a file Vite cannot resolve and Vite says so on every build. Moving the tag into the plugin that already handles the other file:// concerns keeps the emitted HTML byte-identical while removing the warning, and puts everything file:// needs in one place. It stays undeferred so it still sets window.archiveData before the entry script runs. Re-verified by loading the built archive over file:// with seeded data: title, tweet body, Bootstrap layout and fonts all intact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adding an X account on Electron 44 failed at login with "ct0 is null", then crashed the getProgressInfo handler. getCookie read a map that handleCookieTracking filled by watching the Cookie header go past on outgoing requests. Chromium stopped exposing that header to webRequest in Electron 44, so the map was always empty. Probed both runtimes with the same listener: the header is visible on 41.10.7 with Chromium 146 and null on 44.4.3 with Chromium 152. Asking session.cookies.get is the fix, and it also removes the ordering dependency the old approach carried: the map only held a cookie after some request had already carried it, so the very first lookup on a newly added account was never going to succeed. That leaves X's tracking map with no readers, so it goes, and the base class keeps a no-op default for Facebook, which still harvests headers and is still affected. getAccountDataPath gains a username guard. Login failing this way leaves an account with no username, and path.join threw on it, turning a handled error into a crash that hid the real one. initDB already gives up on an empty path. The suite could not have caught this: the mock supplies a Cookie header, so the tests described a browser that no longer exists. The new tests seed the session's cookie jar and never simulate a request, and they fail against the old implementation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Missed in the earlier pass, so every test run still printed the configLoader warning. Its coverage exclude also still named vite.*.config.ts, which stopped matching when those were renamed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
Facebook's side of the cookie regression is tracked separately in #720. It turned out not to need a port: nothing reads Facebook's cookie map, so there is no user-visible breakage there, only dead code that also stopped working. Left out of this PR deliberately. (This was written by an LLM.) |
sleep is async, and four call sites called it without await, so none of them waited. The most consequential is in loadURL, whose own comment says what it is for: "Sleep 2 seconds after loading each URL, to make everything more stable." That settle never happened, and loadURL returned roughly two seconds earlier than every caller has been written to expect. The other three are retry backoffs -- two in loadURL's catch, one in graphqlGetViewerUser -- which meant a failed load retried immediately instead of after a second, and burned all three tries in the time one was supposed to take. The test gates the settle specifically, since that is the one with a behavioural contract. It flushes a macrotask rather than a microtask before asserting: with only a microtask the promise chain has not settled yet and the assertion passes whether or not the sleep is awaited, which is how the first version of this test passed against the unfixed code. Found while investigating the archive SIGSEGV in #721. It is not the cause of that crash -- restoring the settle leaves a byte-identical stack -- so this stands on its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Renaming the four Vite configs and vitest.config to .mts left six entries naming files that no longer exist: three ignores in eslint.config.mjs and three excludes in src/renderer/tsconfig.json. Deleted rather than repointed at .mts, because repointing would imply they do something. They do not, and neither did the .ts versions by the end. eslint only lints what a files pattern matches, and the TypeScript block matches **/*.ts, which does not match .mts -- eslint processes zero .mts files out of the 580 it walks. TypeScript is the same: with all three excludes removed, --listFiles still pulls in no config file, because an include of ./**/*.ts does not match .mts either. So the four .mts configs are linted by nothing and typechecked by nothing. That was already true before this commit and is left alone here; it is a gap worth closing separately, not by keeping dead entries around to suggest otherwise. Prettier does still cover them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The script wanted the private-use callback URL, but that is not what a person has in front of them. The authorization server sends the browser to its own redirect endpoint, which carries the callback in redirect_uri and the OAuth response -- iss, state, code -- beside it. That endpoint URL is what sits in the address bar when the handoff fails. Assembling the callback from it by hand invites joining the response to the path with an ampersand instead of a question mark. There is no query string then, the whole tail parses as one long pathname, it matches no route, and the app reports "Invalid Cyd URL" with no hint as to which part is wrong. So take the redirect URL and do what the browser would have done. A URL carrying no redirect_uri is passed through untouched, which covers a callback that was already assembled correctly, and so is a string that is not a URL at all, so a typo still reaches the app's own error rather than throwing out of the script. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
openCydURL says only the path is ever logged or shown, because the query string can carry an OAuth authorization code. It did that by clearing url.search, which does nothing when there is no query string to clear. Joining the parameters to the path with "&" instead of "?" is exactly that case: the whole tail parses as pathname, so clearing search removes nothing and the code was written to the log and displayed in a dialog in full. Cutting the pathname at the first "&" restores the invariant the comment claims. The same cut says what went wrong. Landing on the error with parameters buried in the path means the route was right and only the separator was not, so the dialog now names the character to fix. It stays quiet for a genuinely unknown path, where there is nothing specific to say. Routing is left strict on both routes. Matching the truncated path would accept a mangled callback and then resolve no flow from its empty query, reporting that the sign-in link had already been used -- which is worse than reporting the real problem. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Brings the dependency tree up to date, takes Electron off an end-of-support line, and moves the archive site off a build tool that stopped shipping releases.
All 24 remaining advisories are one package:
extract-zip, which is vulnerable at every published version and reaches us through@electron/packager. Build-time only, and there is nothing to upgrade to.Commits, in the order they should be read
Each is independently green, so a bad one can be reverted alone.
allowScriptsre-approval they invalidate.@electron/osx-sign,@electron/rebuild.@atproto, leaves. Includes a Prettier reformat of three files.npm audit fixupdate the transitive pins — lockfile only, 16 advisories.allowScriptswith what the tree installs — prunes two dead pins the majors left behind.configLoaderwarnings and a rolldown deprecation.Warnings from the Vite bump
Vite 8.0.16 → 8.3.0 made
npm startnoisy. Confirmed against the published tarballs rather than assumed: theconfigLoaderwarning string is present in 8.3.0 and absent from 8.0.16, and theinlineDynamicImportsdeprecation comes from rolldown 1.2.x, which 8.3.0 pulls in place of 1.0.3.The four Vite configs were ESM loaded as CommonJS, which Vite warns will break when
configLoader: 'native'becomes the default. They are now.mts, which pulled in the three things native ESM always wants: an import attribute on thepackage.jsonimport, a full specifier on the relative import, andimport.meta.dirnamefor__dirname.forge.config.tsstays CommonJS.npm startnow launches with no Vite warnings at all — only Node's pre-existingpunycodedeprecation, which predates this branch.Things worth a reviewer's attention
allowScriptspins install-script approval to an exactname@version. Every bump of Electron or better-sqlite3 silently invalidates its entry, the script never runs, and the Electron binary disappears — which fails the entire suite at once, since the tests run inside that binary. Approving a script does not re-run it either; that needsnpm rebuild <pkg>.npm audit fixdid more than every direct upgrade combined (16 advisories against 5), because nearly all of them sit in transitive packages. It needs--allow-git=all, since Forge resolves@electron/node-gypfrom a git URL. It was never run with--force: npm's proposed "fix" there is to take@electron-forge/clifrom 7.11.2 back to 7.6.1, and Vue CLI back to 3.12.1."fixAvailable": falsedoes not mean no patched release exists — only that npm cannot reach one by bumping a direct dependency.tar,tmpandimage-sizeall had safe versions on the registry. The interesting one ismhtml2html, which pinsjsdom ^15.1.1and dragged inrequest,form-data,qsandtough-cookie— two of the three criticals.archive.tspasses its ownparseDOMbuilt from the root jsdom, so the bundled copy was never doing any work.better-sqlite3 went first on purpose. v13 is built on N-API and ships portable prebuilds, so it survived the ABI jump from 145 to 149 with no rebuild. That also retires the glibc question: the prebuild needs glibc 2.34, under
node:22-bookworm's 2.36. The 2.41 figure in the release notes applies to the old per-ABI assets.Electron 44 needed one override.
node-abionly knew Electron through 43, soelectron-rebuildfailed outright — andpostinstallruns it and exits non-zero.The x-archive migration fixed a bug that would have shipped. Archives are read by double-clicking
index.html, and browsers refuse to fetch ES modules overfile://. Vite tags its entry as a module whatever the output format, sovite.config.tsrewrites it to a classic script — and then it needsdefer, because a classic script in<head>runs before#appis parsed and mounts onto nothing. Neither failure is visible tonpm testor to the build; both were caught by loading the built archive overfile://with seededwindow.archiveDataand asserting on the rendered title, body text, computed layout and fonts.Verification
Green after every commit:
npm test(119 files, 1499 tests),npm run lint,npm run build --workspace=x-archive, a better-sqlite3 smoke test under the Electron binary, andnpx electron-forge package.Verified in Docker on the real release path — Node 24 LTS, npm 12, glibc 2.36 — producing all three Linux distributables:
out/make/zip/linux/x64/out/make/rpm/x64/out/make/deb/x64/Still worth a look before release: Electron 44 drops macOS 12, and the macOS and Windows makers are unexercised here.
A clean install was broken on npm 12
Worth calling out separately, because it predates this branch and is invisible until you clone fresh.
Forge pins
@electron/rebuild@3.7.2, which resolves node-gyp from a git URL. npm from 12 refuses git fetches by default, sonpm installfails withEALLOWGIT. It appears to work for anyone who already has the package unpacked innode_modules, which is why nobody hit it.@electron/rebuild@4.2.0takes node-gyp from the registry, so an override removes the git dependency rather than reopening git fetching to work around one transitive. It drops 102 packages, dedupes the two copies Forge and the root each carried, and supersedes thenode-abioverride added for Electron 44 — two overrides become one.Release toolchain
The image was Node 22 with npm 10, two runtime lines behind and a major behind the npm developers run. The npm gap was not cosmetic: npm 12 records a
libcfield on optional platform packages that npm 10 silently strips, so every release build quietly rewrotepackage-lock.jsonand every developer rewrote it back.Bookworm stays on purpose.
postinstallrebuilds better-sqlite3 from source against the image's glibc, so the base image sets the floor for every Linux user — bookworm is 2.36, trixie would make it 2.41. Node 24 LTS is available on bookworm, so the runtime moves and the floor does not.