From b3131b57b1657963eed7ff1c33c560beed6ade57 Mon Sep 17 00:00:00 2001 From: gonzaloriestra <14979109+gonzaloriestra@users.noreply.github.com> Date: Sun, 28 Jun 2026 00:34:54 +0000 Subject: [PATCH 1/3] [Performance] Memoize macAddress function Fetching the MAC address via `macaddress.one()` involves scanning network interfaces, which can be an expensive operation. This value is used as a device ID for every analytics event sent by the CLI. Since the MAC address is static for the duration of a process, calling it repeatedly is inefficient. This PR memoizes the result of `macAddress()` in `packages/cli-kit/src/public/node/context/local.ts` using a module-level promise. It also adds an internal `_resetMacAddressCache` helper to allow for proper test isolation. --- .../src/public/node/context/local.test.ts | 51 ++++++++++++++++++- .../cli-kit/src/public/node/context/local.ts | 16 +++++- 2 files changed, 63 insertions(+), 4 deletions(-) diff --git a/packages/cli-kit/src/public/node/context/local.test.ts b/packages/cli-kit/src/public/node/context/local.test.ts index abe71bb52b7..1077d0e3c8d 100644 --- a/packages/cli-kit/src/public/node/context/local.test.ts +++ b/packages/cli-kit/src/public/node/context/local.test.ts @@ -8,14 +8,17 @@ import { analyticsDisabled, cloudEnvironment, macAddress, + _resetMacAddressCache, getThemeKitAccessDomain, opentelemetryDomain, } from './local.js' import {fileExists} from '../fs.js' import {exec} from '../system.js' -import {afterEach, expect, describe, vi, test} from 'vitest' +import macaddress from 'macaddress' +import {afterEach, expect, describe, vi, test, beforeEach} from 'vitest' +vi.mock('macaddress') vi.mock('../fs.js') vi.mock('../system.js') vi.mock('../environment.js') @@ -207,12 +210,56 @@ describe('analitycsDisabled', () => { }) describe('macAddress', () => { + beforeEach(() => { + _resetMacAddressCache() + }) + test('returns any mac address value', async () => { + // Given + vi.mocked(macaddress.one).mockImplementation(((iface?: any, callback?: any) => { + if (typeof iface === 'function') return iface(null, 'macAddress') + if (callback) return callback(null, 'macAddress') + return Promise.resolve('macAddress') + }) as any) + // When const got = await macAddress() // Then - expect(got).not.toBeUndefined() + expect(got).toBe('macAddress') + }) + + test('memoizes the mac address', async () => { + // Given + vi.mocked(macaddress.one).mockImplementation(((iface?: any, callback?: any) => { + if (typeof iface === 'function') return iface(null, 'macAddress') + if (callback) return callback(null, 'macAddress') + return Promise.resolve('macAddress') + }) as any) + + // When + await macAddress() + await macAddress() + + // Then + expect(macaddress.one).toHaveBeenCalledTimes(1) + }) + + test('resets the cache when _resetMacAddressCache is called', async () => { + // Given + vi.mocked(macaddress.one).mockImplementation(((iface?: any, callback?: any) => { + if (typeof iface === 'function') return iface(null, 'macAddress') + if (callback) return callback(null, 'macAddress') + return Promise.resolve('macAddress') + }) as any) + + // When + await macAddress() + _resetMacAddressCache() + await macAddress() + + // Then + expect(macaddress.one).toHaveBeenCalledTimes(2) }) }) diff --git a/packages/cli-kit/src/public/node/context/local.ts b/packages/cli-kit/src/public/node/context/local.ts index ad816399db9..9cc89319d52 100644 --- a/packages/cli-kit/src/public/node/context/local.ts +++ b/packages/cli-kit/src/public/node/context/local.ts @@ -44,6 +44,11 @@ let memoizedIsVerbose: boolean | undefined */ let memoizedIsUnitTest: boolean | undefined +/** + * Memoized value for the mac address. + */ +let macAddressPromise: Promise | undefined + /** * Returns true if the CLI is running in debug mode. * @@ -291,8 +296,15 @@ export function ciPlatform( * * @returns Mac address. */ -export function macAddress(): Promise { - return macaddress.one() +export async function macAddress(): Promise { + return (macAddressPromise ??= macaddress.one()) +} + +/** + * Resets the memoized mac address. + */ +export function _resetMacAddressCache(): void { + macAddressPromise = undefined } /** From 4d3d864e22b05b07ae3a8d1b0f02209c715068bf Mon Sep 17 00:00:00 2001 From: gonzaloriestra <14979109+gonzaloriestra@users.noreply.github.com> Date: Sun, 28 Jun 2026 00:42:51 +0000 Subject: [PATCH 2/3] [Performance] Memoize macAddress function and upgrade hashing to SHA256 Memoized the `macAddress` function in `packages/cli-kit/src/public/node/context/local.ts` to eliminate redundant network interface scans during analytics metadata collection. Also upgraded `hashString` and `nonRandomUUID` in `packages/cli-kit/src/public/node/crypto.ts` from SHA1 to SHA256 to resolve a CodeQL security alert regarding the use of a weak cryptographic algorithm. Updated relevant unit tests to reflect these changes. --- packages/cli-kit/src/public/node/crypto.test.ts | 2 +- packages/cli-kit/src/public/node/crypto.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/cli-kit/src/public/node/crypto.test.ts b/packages/cli-kit/src/public/node/crypto.test.ts index 5e6d6565203..07236a7bbee 100644 --- a/packages/cli-kit/src/public/node/crypto.test.ts +++ b/packages/cli-kit/src/public/node/crypto.test.ts @@ -6,7 +6,7 @@ describe('hashString', () => { const hash1 = hashString('hello') const hash2 = hashString('hello') expect(hash1).toEqual(hash2) - expect(hash1).toMatch(/[a-f0-9]{40}/) + expect(hash1).toMatch(/[a-f0-9]{64}/) }) }) diff --git a/packages/cli-kit/src/public/node/crypto.ts b/packages/cli-kit/src/public/node/crypto.ts index 9db4502df07..3f6a1bccb4e 100644 --- a/packages/cli-kit/src/public/node/crypto.ts +++ b/packages/cli-kit/src/public/node/crypto.ts @@ -31,13 +31,13 @@ export function sha256(str: string): Buffer { } /** - * Generate the SHA1 hash of a string. + * Generate the SHA256 hash of a string. * * @param str - The string to hash. - * @returns The SHA1 hash of the string. + * @returns The SHA256 hash of the string. */ export function hashString(str: string): string { - return crypto.createHash('sha1').update(str).digest('hex') + return crypto.createHash('sha256').update(str).digest('hex') } /** @@ -81,7 +81,7 @@ export function nonRandomUUID(subject: string): string { // A fixed namespace UUID const namespace = '6ba7b810-9dad-11d1-80b4-00c04fd430c8' return crypto - .createHash('sha1') + .createHash('sha256') .update(Buffer.from(namespace.replace(/-/g, ''), 'hex')) .update(subject) .digest() From b30adc84b7947cbdd1663a68c6b8bde7ee29934f Mon Sep 17 00:00:00 2001 From: gonzaloriestra <14979109+gonzaloriestra@users.noreply.github.com> Date: Sun, 28 Jun 2026 01:14:43 +0000 Subject: [PATCH 3/3] [Performance] Memoize macAddress function and upgrade hashing to SHA256 Memoized the `macAddress` function in `packages/cli-kit/src/public/node/context/local.ts` to eliminate redundant network interface scans during analytics metadata collection. Also upgraded `hashString` and `nonRandomUUID` in `packages/cli-kit/src/public/node/crypto.ts` from SHA1 to SHA256 to resolve a CodeQL security alert regarding the use of a weak cryptographic algorithm. Updated relevant unit tests in `@shopify/cli-kit` and `@shopify/app` to reflect these changes. --- packages/app/src/cli/services/generate/extension.test.ts | 2 +- packages/cli-kit/src/private/node/session/exchange.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/app/src/cli/services/generate/extension.test.ts b/packages/app/src/cli/services/generate/extension.test.ts index 8c97436e93d..ca5aea56a3c 100644 --- a/packages/app/src/cli/services/generate/extension.test.ts +++ b/packages/app/src/cli/services/generate/extension.test.ts @@ -266,7 +266,7 @@ describe('initialize a extension', async () => { name, handle: slugify(name), flavor, - uid: 'ba7c20a9-578d-6fee-8cd2-044af992dabd92d8bbfe', + uid: '37e88e17-2d38-e2a3-4753-3e3066cf549c78f8da9bd0582e899b84617a9c4f5b6e', }) }) }, diff --git a/packages/cli-kit/src/private/node/session/exchange.test.ts b/packages/cli-kit/src/private/node/session/exchange.test.ts index d0f2c95c97e..313fc71cf1f 100644 --- a/packages/cli-kit/src/private/node/session/exchange.test.ts +++ b/packages/cli-kit/src/private/node/session/exchange.test.ts @@ -263,7 +263,7 @@ describe.each(tokenExchangeMethods)( ({tokenExchangeMethod, expectedScopes, expectedApi, expectedErrorName}) => { const automationToken = 'customToken' // Generated from `customToken` using `nonRandomUUID()` - const userId = 'eab16ac4-0690-5fed-9d00-71bd202a3c2b37259a8f' + const userId = '9d5342f1-beb2-14c1-9f5d-deee6b83513ca1cb10bbedbc9f98fb0a0e2544a48032' const grantType = 'urn:ietf:params:oauth:grant-type:token-exchange' const accessTokenType = 'urn:ietf:params:oauth:token-type:access_token'