From 2c6b233ba7852fcf0223c79c3fdb4e1892806c4b Mon Sep 17 00:00:00 2001 From: gonzaloriestra <14979109+gonzaloriestra@users.noreply.github.com> Date: Sat, 4 Jul 2026 00:25:39 +0000 Subject: [PATCH] Harden random selection in takeRandomFromArray - Replace Math.random() with globalThis.crypto.getRandomValues() - Implement rejection sampling to avoid modulo bias - Add safety guard for empty arrays - Update return type to T | undefined to reflect empty array possibility - Add unit tests for new behavior --- .../cli-kit/src/public/common/array.test.ts | 23 ++++++++++++++++++- packages/cli-kit/src/public/common/array.ts | 20 ++++++++++++++-- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/packages/cli-kit/src/public/common/array.test.ts b/packages/cli-kit/src/public/common/array.test.ts index 720becbac0b..74b84ce42a4 100644 --- a/packages/cli-kit/src/public/common/array.test.ts +++ b/packages/cli-kit/src/public/common/array.test.ts @@ -1,6 +1,27 @@ -import {difference, uniq, uniqBy} from './array.js' +import {difference, takeRandomFromArray, uniq, uniqBy} from './array.js' import {describe, test, expect} from 'vitest' +describe('takeRandomFromArray', () => { + test('returns undefined for an empty array', () => { + // When + const got = takeRandomFromArray([]) + + // Then + expect(got).toBeUndefined() + }) + + test('returns a random element from the array', () => { + // Given + const array = [1, 2, 3, 4, 5] + + // When + const got = takeRandomFromArray(array) + + // Then + expect(array).toContain(got) + }) +}) + describe('uniqBy', () => { test('removes duplicates', () => { // When diff --git a/packages/cli-kit/src/public/common/array.ts b/packages/cli-kit/src/public/common/array.ts index 8b22c0b7b93..00fb1ed9431 100644 --- a/packages/cli-kit/src/public/common/array.ts +++ b/packages/cli-kit/src/public/common/array.ts @@ -8,8 +8,24 @@ import type {List, ValueIteratee} from 'lodash' * @param array - Array from which we'll select a random item. * @returns A random element from the array. */ -export function takeRandomFromArray(array: T[]): T { - return array[Math.floor(Math.random() * array.length)]! +export function takeRandomFromArray(array: T[]): T | undefined { + if (array.length === 0) { + return undefined + } + + // Rejection sampling to avoid modulo bias + const maxUint32 = 0xffffffff + const range = array.length + const limit = maxUint32 - (maxUint32 % range) + + const randomBuffer = new Uint32Array(1) + let randomValue: number + do { + globalThis.crypto.getRandomValues(randomBuffer) + randomValue = randomBuffer[0]! + } while (randomValue >= limit) + + return array[randomValue % range] } /**