Repository navigation
[core] Fix vulnerable nanoid pin by dropping the dependency #4335
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
60588df
af4207f
2a23e1a
d8bd20c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@workflow/core": patch | ||
| --- | ||
|
|
||
| Security: remove the `nanoid` dependency (pinned at 5.1.6, affected by [GHSA-28wg-ghj8-5hjv](https://github.com/advisories/GHSA-28wg-ghj8-5hjv)). Default hook tokens now come from a built-in copy of the same generator, so tokens and seeded random values stay identical for runs recorded on earlier versions. Upgrading `nanoid` itself would have changed them, because `nanoid@5.1.16` draws a different number of random bytes per id. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import seedrandom from 'seedrandom'; | ||
| import { describe, expect, it } from 'vitest'; | ||
| import { createSeededNanoid } from './seeded-nanoid.js'; | ||
|
|
||
| describe('createSeededNanoid', () => { | ||
| // Golden values recorded with nanoid@5.1.6's | ||
| // customRandom(urlAlphabet, 21, ...), the generator that earlier versions | ||
| // used for default hook tokens. | ||
| it('matches the ids and PRNG consumption of nanoid@5.1.6', () => { | ||
| const rng = seedrandom('wrun_test:seed'); | ||
| const generate = createSeededNanoid(rng); | ||
|
|
||
| expect(generate()).toBe('pVjvQyygUGbIid9mUBIhp'); | ||
| expect(generate()).toBe('mgNKWnMBr9yywfW_syBGT'); | ||
| // The next value workflow code would see from Math.random(). | ||
| expect(rng()).toBe(0.3399971329395688); | ||
| }); | ||
|
|
||
| it('draws 34 values per id', () => { | ||
| let draws = 0; | ||
| const generate = createSeededNanoid(() => { | ||
| draws++; | ||
| return 0.5; | ||
| }); | ||
|
|
||
| expect(generate()).toHaveLength(21); | ||
| expect(draws).toBe(34); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| /** | ||
| * Deterministic generator for default hook tokens inside workflow code. | ||
| * | ||
| * The output, and the number of values drawn from `random`, are part of the | ||
| * replay contract. Every draw advances the run-seeded PRNG that also feeds | ||
| * `Math.random()` and correlation IDs in workflow code, so a different | ||
| * consumption pattern changes every ID minted after the first `createHook()`, | ||
| * and a run recorded on one version diverges when it is replayed on another. | ||
| * | ||
| * This reproduces `nanoid@5.1.6`'s `customRandom(urlAlphabet, 21, ...)` | ||
| * exactly, instead of depending on nanoid, whose `customRandom` changed how | ||
| * many bytes it requests in 5.1.16. Do not change it. | ||
| */ | ||
| const URL_ALPHABET = | ||
| 'useandom-26T198340PX75pxJACKVERYMINDBUSHWOLF_GQZbfghjklqvwyzrict'; | ||
| const ID_SIZE = 21; | ||
| // nanoid 5.1.6 requests ceil(1.6 * mask * size / alphabet.length) bytes per | ||
| // batch: ceil(1.6 * 63 * 21 / 64) = 34. With a 64-character alphabet every | ||
| // byte maps to a character, so one batch always yields a full id, read from | ||
| // the end of the batch. | ||
| const BYTES_PER_ID = 34; | ||
|
|
||
| export function createSeededNanoid(random: () => number): () => string { | ||
| return () => { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AI Review: Note Checked this against the real |
||
| const bytes = new Uint8Array(BYTES_PER_ID).map(() => 256 * random()); | ||
| let id = ''; | ||
| for (let i = BYTES_PER_ID - 1; id.length < ID_SIZE; i--) { | ||
| id += URL_ALPHABET[bytes[i] & 63]; | ||
| } | ||
| return id; | ||
| }; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AI Review: Nit
The changeset is used verbatim in the changelog, and the repo convention is one terse line with no implementation detail. Something like:
Security: remove the nanoid dependency (GHSA-28wg-ghj8-5hjv). Hook tokens are unchanged for existing runs.The rationale for not bumping fits better in the PR description, where it already is.