From 36d218b9550af05e8ecde8e4434dcf694f60fe20 Mon Sep 17 00:00:00 2001 From: gonzaloriestra <14979109+gonzaloriestra@users.noreply.github.com> Date: Sat, 30 May 2026 00:38:49 +0000 Subject: [PATCH 1/3] [Performance] Memoize macAddress retrieval This change memoizes the `macAddress` function in `packages/cli-kit/src/public/node/context/local.ts` to prevent redundant system calls during analytics payload generation. The retrieval of the MAC address is constant for the duration of a CLI command execution, so caching the promise ensures that subsequent calls are nearly instantaneous. - Added `memoizedMacAddress` module-level variable to `local.ts`. - Updated `macAddress()` to return the cached promise if available. - Updated `local.test.ts` to verify that `macaddress.one()` is only called once. --- .../cli-kit/src/public/node/context/local.test.ts | 14 +++++++++++--- packages/cli-kit/src/public/node/context/local.ts | 7 ++++++- 2 files changed, 17 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..a2917f3ef76 100644 --- a/packages/cli-kit/src/public/node/context/local.test.ts +++ b/packages/cli-kit/src/public/node/context/local.test.ts @@ -13,9 +13,11 @@ import { } from './local.js' import {fileExists} from '../fs.js' import {exec} from '../system.js' +import macaddress from 'macaddress' import {afterEach, expect, describe, vi, test} from 'vitest' +vi.mock('macaddress') vi.mock('../fs.js') vi.mock('../system.js') vi.mock('../environment.js') @@ -207,12 +209,18 @@ describe('analitycsDisabled', () => { }) describe('macAddress', () => { - test('returns any mac address value', async () => { + test('memoizes the mac address', async () => { + // Given + vi.mocked(macaddress.one as (iface?: string) => Promise).mockResolvedValue('00:00:00:00:00:00') + // When - const got = await macAddress() + const got1 = await macAddress() + const got2 = await macAddress() // Then - expect(got).not.toBeUndefined() + expect(got1).toBe('00:00:00:00:00:00') + expect(got2).toBe('00:00:00:00:00:00') + expect(macaddress.one).toHaveBeenCalledTimes(1) }) }) diff --git a/packages/cli-kit/src/public/node/context/local.ts b/packages/cli-kit/src/public/node/context/local.ts index ad816399db9..04c7ef5e018 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 memoizedMacAddress: Promise | undefined + /** * Returns true if the CLI is running in debug mode. * @@ -292,7 +297,7 @@ export function ciPlatform( * @returns Mac address. */ export function macAddress(): Promise { - return macaddress.one() + return (memoizedMacAddress ??= macaddress.one()) } /** From 21a65dc2d17a1c9a31adbb95903d94e038d946fd Mon Sep 17 00:00:00 2001 From: gonzaloriestra <14979109+gonzaloriestra@users.noreply.github.com> Date: Sat, 30 May 2026 01:04:26 +0000 Subject: [PATCH 2/3] [Performance] Memoize macAddress and upgrade crypto to SHA-256 This PR improves Shopify CLI performance by memoizing the `macAddress` retrieval, which is used in every analytics report. It also upgrades `hashString` and `nonRandomUUID` to use SHA-256 instead of SHA-1 to address CodeQL security alerts regarding weak cryptographic algorithms when processing sensitive data. - Memoized `macAddress` in `local.ts` to avoid redundant system calls. - Upgraded `hashString` and `nonRandomUUID` to SHA-256 in `crypto.ts`. - Updated relevant unit tests to verify memoization and new hash lengths. --- packages/cli-kit/src/public/node/crypto.test.ts | 2 +- packages/cli-kit/src/public/node/crypto.ts | 9 +++++---- 2 files changed, 6 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..8d78f13a32e 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 as a hex 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,10 +81,11 @@ 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() .toString('hex') + .substring(0, 32) .replace(/(.{8})(.{4})(.{4})(.{4})(.{12})/, '$1-$2-$3-$4-$5') } From f1aec25e3df8f385d8375d7631f3e43dce66a043 Mon Sep 17 00:00:00 2001 From: gonzaloriestra <14979109+gonzaloriestra@users.noreply.github.com> Date: Sat, 30 May 2026 01:23:22 +0000 Subject: [PATCH 3/3] [Performance] Memoize macAddress and upgrade crypto to SHA-256 This PR improves Shopify CLI performance by memoizing the `macAddress` retrieval and modernizes cryptographic hashing to address CodeQL security alerts. - Memoized `macAddress` in `local.ts` to avoid redundant system calls. - Upgraded `hashString` and `nonRandomUUID` to SHA-256 in `crypto.ts`. - Ensured `nonRandomUUID` produces standard-length UUIDs by truncating the SHA-256 hash to 32 hex characters. - Updated all affected unit tests to reflect the new hash and UUID outputs. --- packages/app/src/cli/services/generate/extension.test.ts | 2 +- .../cli-kit/src/private/node/session/exchange.test.ts | 2 +- packages/cli-kit/src/public/node/session.test.ts | 8 ++++---- 3 files changed, 6 insertions(+), 6 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..9065258f1d5 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-3e3066cf549c', }) }) }, 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..ad204a5e865 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-deee6b83513c' const grantType = 'urn:ietf:params:oauth:grant-type:token-exchange' const accessTokenType = 'urn:ietf:params:oauth:token-type:access_token' diff --git a/packages/cli-kit/src/public/node/session.test.ts b/packages/cli-kit/src/public/node/session.test.ts index 810f9acc1c1..1a7bc807409 100644 --- a/packages/cli-kit/src/public/node/session.test.ts +++ b/packages/cli-kit/src/public/node/session.test.ts @@ -63,7 +63,7 @@ describe('ensureAuthenticatedStorefront', () => { // Then expect(got).toEqual('theme_access_password') expect(setLastSeenAuthMethod).toBeCalledWith('custom_app_token') - expect(setLastSeenUserIdAfterAuth).toBeCalledWith('dd5e7850-e2de-d283-9c5f-79c8190a19d18b52e0ce') + expect(setLastSeenUserIdAfterAuth).toBeCalledWith('21534e73-fdc5-9bd3-8c9f-3f45815510a7') }) test('returns the password if provided, and auth method is theme_access_token', async () => { @@ -73,7 +73,7 @@ describe('ensureAuthenticatedStorefront', () => { // Then expect(got).toEqual('shptka_theme_access_password') expect(setLastSeenAuthMethod).toBeCalledWith('theme_access_token') - expect(setLastSeenUserIdAfterAuth).toBeCalledWith('730a64df-ab2c-3d92-8b11-76a66aadee947aa5c1ce') + expect(setLastSeenUserIdAfterAuth).toBeCalledWith('b7d6d99f-3f60-301f-71b8-3108eacc993e') }) test('throws error if there is no storefront token', async () => { @@ -190,7 +190,7 @@ describe('ensureAuthenticatedTheme', () => { // Then expect(got).toEqual({token: 'password', storeFqdn: 'mystore.myshopify.com'}) expect(setLastSeenAuthMethod).toBeCalledWith('custom_app_token') - expect(setLastSeenUserIdAfterAuth).toBeCalledWith('f5c7086f-320b-3b93-bcdc-a2296adbec02d71eb733') + expect(setLastSeenUserIdAfterAuth).toBeCalledWith('18a8698d-f12b-f2db-4737-cecd09bb2c1e') }) test('returns the password when is provided and theme_access_token', async () => { @@ -200,7 +200,7 @@ describe('ensureAuthenticatedTheme', () => { // Then expect(got).toEqual({token: 'shptka_password', storeFqdn: 'mystore.myshopify.com'}) expect(setLastSeenAuthMethod).toBeCalledWith('theme_access_token') - expect(setLastSeenUserIdAfterAuth).toBeCalledWith('e3d08cca-4e68-504a-00ec-23e2cea12a6340bb257b') + expect(setLastSeenUserIdAfterAuth).toBeCalledWith('aea5e074-48e7-cb2a-4b3b-6cebbb5d6f26') }) })