Update glob to v10.5 to fix CVE-2025-64756 - #239
Conversation
There was a problem hiding this comment.
Please rebase with master, update the package-lock.json and I will approve. @guaycuru
|
@pablotransifex Done! |
Looks ok, just my bad please remove the Release will be conducted on Monday most probably. Thanks for the contribution by the way! @guaycuru |
This reverts commit 3ebed70.
|
No worries, |
There was a problem hiding this comment.
Thanks for this — and it's more important than the title suggests. I reproduced what this actually fixes: on glob v9+ the old callback-based call silently no-ops. The pre-change code wraps glob's v8 callback in a Promise; since v9 removed that callback API, the callback never fires, the await never settles, and Node exits 0 having parsed and uploaded nothing:
Parsing all files to detect translatable content...
---- process exited with code: 0 ---- (glob@11, pre-PR code — "files found" line never runs)
That's a silent push failure — no strings reach Transifex.
One correction on the CVE framing, since it affects how this gets described: I checked npm's advisory DB and OSV, and glob@8.1.0 is not affected by CVE-2025-64756 — the advisory only covers >=10.2.0 <10.5.0 and >=11.0.0 <11.1.0. The vulnerability is in glob's CLI (-c/--cmd, run with shell:true); this package uses glob's library API only and never invokes its CLI, so the shipped code was never exposed. npm audit on a glob@8 tree reports 0 vulnerabilities.
So I'd suggest rewording around the actual fix — something like "Migrate to glob's promise API (fixes silent no-op push on glob v9+)" or "drop deprecated glob@8" — with the body noting the callback API was removed in glob v9 and causes push to upload nothing and exit 0 rather than fixing CVE-2025-64756.
Two smaller notes:
- Please add .sort() to the result (see inline comment) — glob v9+ dropped default sorting, which makes the pushed payload order-dependent.
- glob@10.5.0 is itself still marked deprecated upstream (only 13.x is current), and going higher isn't free — glob 13 requires Node 18+, which conflicts with this package's engines.node: ">=16.0.0". So the ^10.5.0 pin is a reasonable choice given the Node floor; no objection to it. Happy to approve once .sort() and the description are in.
| return resolve(files); | ||
| }); | ||
| }); | ||
| const allFiles = await glob(filePattern); |
There was a problem hiding this comment.
One change needed before this merges. glob v9 removed the default alphabetical sort of results (and deleted the nosort option that controlled it), so await glob(filePattern) now returns paths in raw filesystem order where glob@8 returned them sorted.
That order feeds mergePayload in src/api/merge.js, where developer_comment and character_limit are last-write-wins across files. So when the same source string appears in two files with different _comment / _charlimit values, which value gets pushed now depends on filesystem order — it can differ between a dev machine and CI, and change between runs after a file is rewritten. occurrences and tags reorder too, which shows up as spurious "Updated strings" on every push even when nothing changed. Unfortunately this whole behavior isn't covered by existing tests.
To restore the prior behavior make a small addition:
const allFiles = (await glob(filePattern)).sort();
We we'll add covering tests on a follow-up PR.
3 participants
This PR updates glob to v10.5 which is no longer vulnerable to CVE-2025-64756