From 16d24c5062c2657dd11a00e59f77f7165685dcd4 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Tue, 2 Jun 2026 17:12:29 +0300 Subject: [PATCH] Validate --port flags via a reusable cli-kit flag `theme dev --port 13245574` (and other out-of-range, negative, non-integer, or non-numeric values) crashed with an unhandled RangeError ("options.port should be >= 0 and < 65536"). The raw value reached server.listen / checkPortAvailability, whose synchronous RangeError (ERR_SOCKET_BAD_PORT) escaped uncaught because only the asynchronous 'error' event was handled. The same unbounded path existed for app dev's port flags (Flags.integer with no min/max). Add a reusable portFlag to @shopify/cli-kit/node/cli built on oclif's Flags.integer constrained to [1, 65535]; it rejects non-integer and out-of-range values at parse time and surfaces the valid range in each flag's --help description. Wire it into every previously-unvalidated port flag: theme dev --port and app dev's --localhost-port, --theme-app-extension-port, and --graphiql-port. As defense-in-depth, checkPortAvailability now catches the synchronous RangeError (resolving false) while rethrowing any other error, and the theme --port is carried as a number through DevServerContext rather than a string. Fixes the crash reported in https://github.com/shop/issues/issues/48221 Co-Authored-By: Claude Opus 4.8 (1M context) --- .changeset/app-dev-port-validation.md | 5 ++ .changeset/strong-pumas-port.md | 5 ++ .changeset/theme-dev-port-validation.md | 5 ++ .../commands/interfaces/app-dev.interface.ts | 4 +- .../interfaces/theme-dev.interface.ts | 2 +- .../generated/generated_docs_data_v2.json | 10 +-- packages/app/src/cli/commands/app/dev.ts | 8 +-- packages/cli-kit/src/public/node/cli.test.ts | 15 +++- packages/cli-kit/src/public/node/cli.ts | 13 ++++ packages/cli-kit/src/public/node/tcp.test.ts | 4 ++ packages/cli-kit/src/public/node/tcp.ts | 13 +++- packages/cli/README.md | 7 +- packages/cli/oclif.manifest.json | 8 +-- packages/theme/src/cli/commands/theme/dev.ts | 4 +- packages/theme/src/cli/services/dev.test.ts | 72 +++++++++++++++++++ packages/theme/src/cli/services/dev.ts | 20 +++--- .../hot-reload/server.test.ts | 2 +- .../utilities/theme-environment/proxy.test.ts | 2 +- .../theme-environment.test.ts | 2 +- .../cli/utilities/theme-environment/types.ts | 2 +- .../theme-ext-environment/theme-ext-server.ts | 2 +- 21 files changed, 167 insertions(+), 38 deletions(-) create mode 100644 .changeset/app-dev-port-validation.md create mode 100644 .changeset/strong-pumas-port.md create mode 100644 .changeset/theme-dev-port-validation.md diff --git a/.changeset/app-dev-port-validation.md b/.changeset/app-dev-port-validation.md new file mode 100644 index 00000000000..ca5f1d5abe2 --- /dev/null +++ b/.changeset/app-dev-port-validation.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': patch +--- + +`app dev` now shows a clear error when `--localhost-port`, `--theme-app-extension-port`, or `--graphiql-port` is given an invalid value. The port must be a number between 1 and 65535. diff --git a/.changeset/strong-pumas-port.md b/.changeset/strong-pumas-port.md new file mode 100644 index 00000000000..b6eabeaf11f --- /dev/null +++ b/.changeset/strong-pumas-port.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': minor +--- + +Add a reusable `--port` flag that validates the value is a number between 1 and 65535, for commands that accept a port. diff --git a/.changeset/theme-dev-port-validation.md b/.changeset/theme-dev-port-validation.md new file mode 100644 index 00000000000..82c0f94f972 --- /dev/null +++ b/.changeset/theme-dev-port-validation.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': patch +--- + +`theme dev` now shows a clear error when `--port` is given an invalid value, instead of crashing. The port must be a number between 1 and 65535. diff --git a/docs-shopify.dev/commands/interfaces/app-dev.interface.ts b/docs-shopify.dev/commands/interfaces/app-dev.interface.ts index cabb8f600ef..82bc41c8b3f 100644 --- a/docs-shopify.dev/commands/interfaces/app-dev.interface.ts +++ b/docs-shopify.dev/commands/interfaces/app-dev.interface.ts @@ -23,7 +23,7 @@ export interface appdev { '-c, --config '?: string /** - * Port to use for localhost. + * Port to use for localhost. Must be between 1 and 65535. * @environment SHOPIFY_FLAG_LOCALHOST_PORT */ '--localhost-port '?: string @@ -83,7 +83,7 @@ export interface appdev { '-t, --theme '?: string /** - * Local port of the theme app extension development server. + * Local port of the theme app extension development server. Must be between 1 and 65535. * @environment SHOPIFY_FLAG_THEME_APP_EXTENSION_PORT */ '--theme-app-extension-port '?: string diff --git a/docs-shopify.dev/commands/interfaces/theme-dev.interface.ts b/docs-shopify.dev/commands/interfaces/theme-dev.interface.ts index d1388b0d3a4..0945a3a9d46 100644 --- a/docs-shopify.dev/commands/interfaces/theme-dev.interface.ts +++ b/docs-shopify.dev/commands/interfaces/theme-dev.interface.ts @@ -95,7 +95,7 @@ export interface themedev { '--path '?: string /** - * Local port to serve theme preview from. + * Local port to serve theme preview from. Must be between 1 and 65535. * @environment SHOPIFY_FLAG_PORT */ '--port '?: string diff --git a/docs-shopify.dev/generated/generated_docs_data_v2.json b/docs-shopify.dev/generated/generated_docs_data_v2.json index 4965084de82..782fa48b708 100644 --- a/docs-shopify.dev/generated/generated_docs_data_v2.json +++ b/docs-shopify.dev/generated/generated_docs_data_v2.json @@ -867,7 +867,7 @@ "syntaxKind": "PropertySignature", "name": "--localhost-port ", "value": "string", - "description": "Port to use for localhost.", + "description": "Port to use for localhost. Must be between 1 and 65535.", "isOptional": true, "environmentValue": "SHOPIFY_FLAG_LOCALHOST_PORT" }, @@ -939,7 +939,7 @@ "syntaxKind": "PropertySignature", "name": "--theme-app-extension-port ", "value": "string", - "description": "Local port of the theme app extension development server.", + "description": "Local port of the theme app extension development server. Must be between 1 and 65535.", "isOptional": true, "environmentValue": "SHOPIFY_FLAG_THEME_APP_EXTENSION_PORT" }, @@ -998,7 +998,7 @@ "environmentValue": "SHOPIFY_FLAG_THEME" } ], - "value": "export interface appdev {\n /**\n * Resource URL for checkout UI extension. Format: \"/cart/{productVariantID}:{productQuantity}\"\n * @environment SHOPIFY_FLAG_CHECKOUT_CART_URL\n */\n '--checkout-cart-url '?: string\n\n /**\n * The Client ID of your app.\n * @environment SHOPIFY_FLAG_CLIENT_ID\n */\n '--client-id '?: string\n\n /**\n * The name of the app configuration.\n * @environment SHOPIFY_FLAG_APP_CONFIG\n */\n '-c, --config '?: string\n\n /**\n * Port to use for localhost.\n * @environment SHOPIFY_FLAG_LOCALHOST_PORT\n */\n '--localhost-port '?: string\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Uses the app URL from the toml file instead an autogenerated URL for dev.\n * @environment SHOPIFY_FLAG_NO_UPDATE\n */\n '--no-update'?: ''\n\n /**\n * The file path or URL. The file path is to a file that you want updated on idle. The URL path is where you want a webhook posted to report on file changes.\n * @environment SHOPIFY_FLAG_NOTIFY\n */\n '--notify '?: string\n\n /**\n * The path to your app directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path '?: string\n\n /**\n * Reset all your settings.\n * @environment SHOPIFY_FLAG_RESET\n */\n '--reset'?: ''\n\n /**\n * Skips the installation of dependencies. Deprecated, use workspaces instead.\n * @environment SHOPIFY_FLAG_SKIP_DEPENDENCIES_INSTALLATION\n */\n '--skip-dependencies-installation'?: ''\n\n /**\n * Store URL. Must be an existing development or Shopify Plus sandbox store.\n * @environment SHOPIFY_FLAG_STORE\n */\n '-s, --store '?: string\n\n /**\n * Resource URL for subscription UI extension. Format: \"/products/{productId}\"\n * @environment SHOPIFY_FLAG_SUBSCRIPTION_PRODUCT_URL\n */\n '--subscription-product-url '?: string\n\n /**\n * Theme ID or name of the theme app extension host theme.\n * @environment SHOPIFY_FLAG_THEME\n */\n '-t, --theme '?: string\n\n /**\n * Local port of the theme app extension development server.\n * @environment SHOPIFY_FLAG_THEME_APP_EXTENSION_PORT\n */\n '--theme-app-extension-port '?: string\n\n /**\n * Use a custom tunnel, it must be running before executing dev. Format: \"https://my-tunnel-url:port\".\n * @environment SHOPIFY_FLAG_TUNNEL_URL\n */\n '--tunnel-url '?: string\n\n /**\n * Service entry point will listen to localhost. A tunnel won't be used. Will work for testing many app features, but not those that directly invoke your app (E.g: Webhooks)\n * @environment SHOPIFY_FLAG_USE_LOCALHOST\n */\n '--use-localhost'?: ''\n\n /**\n * Increase the verbosity of the output.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n}" + "value": "export interface appdev {\n /**\n * Resource URL for checkout UI extension. Format: \"/cart/{productVariantID}:{productQuantity}\"\n * @environment SHOPIFY_FLAG_CHECKOUT_CART_URL\n */\n '--checkout-cart-url '?: string\n\n /**\n * The Client ID of your app.\n * @environment SHOPIFY_FLAG_CLIENT_ID\n */\n '--client-id '?: string\n\n /**\n * The name of the app configuration.\n * @environment SHOPIFY_FLAG_APP_CONFIG\n */\n '-c, --config '?: string\n\n /**\n * Port to use for localhost. Must be between 1 and 65535.\n * @environment SHOPIFY_FLAG_LOCALHOST_PORT\n */\n '--localhost-port '?: string\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Uses the app URL from the toml file instead an autogenerated URL for dev.\n * @environment SHOPIFY_FLAG_NO_UPDATE\n */\n '--no-update'?: ''\n\n /**\n * The file path or URL. The file path is to a file that you want updated on idle. The URL path is where you want a webhook posted to report on file changes.\n * @environment SHOPIFY_FLAG_NOTIFY\n */\n '--notify '?: string\n\n /**\n * The path to your app directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path '?: string\n\n /**\n * Reset all your settings.\n * @environment SHOPIFY_FLAG_RESET\n */\n '--reset'?: ''\n\n /**\n * Skips the installation of dependencies. Deprecated, use workspaces instead.\n * @environment SHOPIFY_FLAG_SKIP_DEPENDENCIES_INSTALLATION\n */\n '--skip-dependencies-installation'?: ''\n\n /**\n * Store URL. Must be an existing development or Shopify Plus sandbox store.\n * @environment SHOPIFY_FLAG_STORE\n */\n '-s, --store '?: string\n\n /**\n * Resource URL for subscription UI extension. Format: \"/products/{productId}\"\n * @environment SHOPIFY_FLAG_SUBSCRIPTION_PRODUCT_URL\n */\n '--subscription-product-url '?: string\n\n /**\n * Theme ID or name of the theme app extension host theme.\n * @environment SHOPIFY_FLAG_THEME\n */\n '-t, --theme '?: string\n\n /**\n * Local port of the theme app extension development server. Must be between 1 and 65535.\n * @environment SHOPIFY_FLAG_THEME_APP_EXTENSION_PORT\n */\n '--theme-app-extension-port '?: string\n\n /**\n * Use a custom tunnel, it must be running before executing dev. Format: \"https://my-tunnel-url:port\".\n * @environment SHOPIFY_FLAG_TUNNEL_URL\n */\n '--tunnel-url '?: string\n\n /**\n * Service entry point will listen to localhost. A tunnel won't be used. Will work for testing many app features, but not those that directly invoke your app (E.g: Webhooks)\n * @environment SHOPIFY_FLAG_USE_LOCALHOST\n */\n '--use-localhost'?: ''\n\n /**\n * Increase the verbosity of the output.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n}" } }, "appenvpull": { @@ -4689,7 +4689,7 @@ "syntaxKind": "PropertySignature", "name": "--port ", "value": "string", - "description": "Local port to serve theme preview from.", + "description": "Local port to serve theme preview from. Must be between 1 and 65535.", "isOptional": true, "environmentValue": "SHOPIFY_FLAG_PORT" }, @@ -4784,7 +4784,7 @@ "environmentValue": "SHOPIFY_FLAG_IGNORE" } ], - "value": "export interface themedev {\n /**\n * Allow development on a live theme.\n * @environment SHOPIFY_FLAG_ALLOW_LIVE\n */\n '-a, --allow-live'?: ''\n\n /**\n * The environment to apply to the current command.\n * @environment SHOPIFY_FLAG_ENVIRONMENT\n */\n '-e, --environment '?: string\n\n /**\n * Controls the visibility of the error overlay when an theme asset upload fails:\n- silent Prevents the error overlay from appearing.\n- default Displays the error overlay.\n \n * @environment SHOPIFY_FLAG_ERROR_OVERLAY\n */\n '--error-overlay '?: string\n\n /**\n * Set which network interface the web server listens on. The default value is 127.0.0.1.\n * @environment SHOPIFY_FLAG_HOST\n */\n '--host '?: string\n\n /**\n * Skip hot reloading any files that match the specified pattern.\n * @environment SHOPIFY_FLAG_IGNORE\n */\n '-x, --ignore '?: string\n\n /**\n * The listing preset to use for multi-preset themes. Applies preset files from listings/[preset-name] directory.\n * @environment SHOPIFY_FLAG_LISTING\n */\n '--listing '?: string\n\n /**\n * The live reload mode switches the server behavior when a file is modified:\n- hot-reload Hot reloads local changes to CSS and sections (default)\n- full-page Always refreshes the entire page\n- off Deactivate live reload\n * @environment SHOPIFY_FLAG_LIVE_RELOAD\n */\n '--live-reload '?: string\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Prevents files from being deleted in the remote theme when a file has been deleted locally. This applies to files that are deleted while the command is running, and files that have been deleted locally before the command is run.\n * @environment SHOPIFY_FLAG_NODELETE\n */\n '-n, --nodelete'?: ''\n\n /**\n * The file path or URL. The file path is to a file that you want updated on idle. The URL path is where you want a webhook posted to report on file changes.\n * @environment SHOPIFY_FLAG_NOTIFY\n */\n '--notify '?: string\n\n /**\n * Hot reload only files that match the specified pattern.\n * @environment SHOPIFY_FLAG_ONLY\n */\n '-o, --only '?: string\n\n /**\n * Automatically launch the theme preview in your default web browser.\n * @environment SHOPIFY_FLAG_OPEN\n */\n '--open'?: ''\n\n /**\n * Password generated from the Theme Access app or an Admin API token.\n * @environment SHOPIFY_CLI_THEME_TOKEN\n */\n '--password '?: string\n\n /**\n * The path where you want to run the command. Defaults to the current working directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path '?: string\n\n /**\n * Local port to serve theme preview from.\n * @environment SHOPIFY_FLAG_PORT\n */\n '--port '?: string\n\n /**\n * Store URL. It can be the store prefix (example) or the full myshopify.com URL (example.myshopify.com, https://example.myshopify.com).\n * @environment SHOPIFY_FLAG_STORE\n */\n '-s, --store '?: string\n\n /**\n * The password for storefronts with password protection.\n * @environment SHOPIFY_FLAG_STORE_PASSWORD\n */\n '--store-password '?: string\n\n /**\n * Theme ID or name of the remote theme.\n * @environment SHOPIFY_FLAG_THEME_ID\n */\n '-t, --theme '?: string\n\n /**\n * Synchronize Theme Editor updates in the local theme files.\n * @environment SHOPIFY_FLAG_THEME_EDITOR_SYNC\n */\n '--theme-editor-sync'?: ''\n\n /**\n * Increase the verbosity of the output.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n}" + "value": "export interface themedev {\n /**\n * Allow development on a live theme.\n * @environment SHOPIFY_FLAG_ALLOW_LIVE\n */\n '-a, --allow-live'?: ''\n\n /**\n * The environment to apply to the current command.\n * @environment SHOPIFY_FLAG_ENVIRONMENT\n */\n '-e, --environment '?: string\n\n /**\n * Controls the visibility of the error overlay when an theme asset upload fails:\n- silent Prevents the error overlay from appearing.\n- default Displays the error overlay.\n \n * @environment SHOPIFY_FLAG_ERROR_OVERLAY\n */\n '--error-overlay '?: string\n\n /**\n * Set which network interface the web server listens on. The default value is 127.0.0.1.\n * @environment SHOPIFY_FLAG_HOST\n */\n '--host '?: string\n\n /**\n * Skip hot reloading any files that match the specified pattern.\n * @environment SHOPIFY_FLAG_IGNORE\n */\n '-x, --ignore '?: string\n\n /**\n * The listing preset to use for multi-preset themes. Applies preset files from listings/[preset-name] directory.\n * @environment SHOPIFY_FLAG_LISTING\n */\n '--listing '?: string\n\n /**\n * The live reload mode switches the server behavior when a file is modified:\n- hot-reload Hot reloads local changes to CSS and sections (default)\n- full-page Always refreshes the entire page\n- off Deactivate live reload\n * @environment SHOPIFY_FLAG_LIVE_RELOAD\n */\n '--live-reload '?: string\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Prevents files from being deleted in the remote theme when a file has been deleted locally. This applies to files that are deleted while the command is running, and files that have been deleted locally before the command is run.\n * @environment SHOPIFY_FLAG_NODELETE\n */\n '-n, --nodelete'?: ''\n\n /**\n * The file path or URL. The file path is to a file that you want updated on idle. The URL path is where you want a webhook posted to report on file changes.\n * @environment SHOPIFY_FLAG_NOTIFY\n */\n '--notify '?: string\n\n /**\n * Hot reload only files that match the specified pattern.\n * @environment SHOPIFY_FLAG_ONLY\n */\n '-o, --only '?: string\n\n /**\n * Automatically launch the theme preview in your default web browser.\n * @environment SHOPIFY_FLAG_OPEN\n */\n '--open'?: ''\n\n /**\n * Password generated from the Theme Access app or an Admin API token.\n * @environment SHOPIFY_CLI_THEME_TOKEN\n */\n '--password '?: string\n\n /**\n * The path where you want to run the command. Defaults to the current working directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path '?: string\n\n /**\n * Local port to serve theme preview from. Must be between 1 and 65535.\n * @environment SHOPIFY_FLAG_PORT\n */\n '--port '?: string\n\n /**\n * Store URL. It can be the store prefix (example) or the full myshopify.com URL (example.myshopify.com, https://example.myshopify.com).\n * @environment SHOPIFY_FLAG_STORE\n */\n '-s, --store '?: string\n\n /**\n * The password for storefronts with password protection.\n * @environment SHOPIFY_FLAG_STORE_PASSWORD\n */\n '--store-password '?: string\n\n /**\n * Theme ID or name of the remote theme.\n * @environment SHOPIFY_FLAG_THEME_ID\n */\n '-t, --theme '?: string\n\n /**\n * Synchronize Theme Editor updates in the local theme files.\n * @environment SHOPIFY_FLAG_THEME_EDITOR_SYNC\n */\n '--theme-editor-sync'?: ''\n\n /**\n * Increase the verbosity of the output.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n}" } }, "themeduplicate": { diff --git a/packages/app/src/cli/commands/app/dev.ts b/packages/app/src/cli/commands/app/dev.ts index fc9e47f9681..17edc962a05 100644 --- a/packages/app/src/cli/commands/app/dev.ts +++ b/packages/app/src/cli/commands/app/dev.ts @@ -7,7 +7,7 @@ import {storeContext} from '../../services/store-context.js' import {getTunnelMode} from '../../services/dev/tunnel-mode.js' import {Flags} from '@oclif/core' import {normalizeStoreFqdn} from '@shopify/cli-kit/node/context/fqdn' -import {globalFlags} from '@shopify/cli-kit/node/cli' +import {globalFlags, portFlag} from '@shopify/cli-kit/node/cli' import {addPublicMetadata} from '@shopify/cli-kit/node/metadata' export default class Dev extends AppLinkedCommand { @@ -57,7 +57,7 @@ export default class Dev extends AppLinkedCommand { default: false, exclusive: ['tunnel-url'], }), - 'localhost-port': Flags.integer({ + 'localhost-port': portFlag({ description: 'Port to use for localhost.', env: 'SHOPIFY_FLAG_LOCALHOST_PORT', }), @@ -66,7 +66,7 @@ export default class Dev extends AppLinkedCommand { description: 'Theme ID or name of the theme app extension host theme.', env: 'SHOPIFY_FLAG_THEME', }), - 'theme-app-extension-port': Flags.integer({ + 'theme-app-extension-port': portFlag({ description: 'Local port of the theme app extension development server.', env: 'SHOPIFY_FLAG_THEME_APP_EXTENSION_PORT', }), @@ -75,7 +75,7 @@ export default class Dev extends AppLinkedCommand { 'The file path or URL. The file path is to a file that you want updated on idle. The URL path is where you want a webhook posted to report on file changes.', env: 'SHOPIFY_FLAG_NOTIFY', }), - 'graphiql-port': Flags.integer({ + 'graphiql-port': portFlag({ hidden: true, description: 'Local port of the GraphiQL development server.', env: 'SHOPIFY_FLAG_GRAPHIQL_PORT', diff --git a/packages/cli-kit/src/public/node/cli.test.ts b/packages/cli-kit/src/public/node/cli.test.ts index 5aa8dc8410b..6082c8dcadc 100644 --- a/packages/cli-kit/src/public/node/cli.test.ts +++ b/packages/cli-kit/src/public/node/cli.test.ts @@ -1,4 +1,4 @@ -import {clearCache, runCLI, runCreateCLI} from './cli.js' +import {clearCache, runCLI, runCreateCLI, portFlag} from './cli.js' import {findUpAndReadPackageJson} from './node-package-manager.js' import {mockAndCaptureOutput} from './testing/output.js' import * as confStore from '../../private/node/conf-store.js' @@ -135,3 +135,16 @@ describe('clearCache', () => { spy.mockRestore() }) }) + +describe('portFlag', () => { + const flag = portFlag() + test('parses a valid port to a number', async () => { + await expect(flag.parse('9292', {} as any, flag as any)).resolves.toBe(9292) + }) + test.each(['13245574', '65536', '0', '-1', 'abc', '92.5', '', '0x10', '1e2', ' 9293 ', '+9292'])( + 'rejects invalid port %s', + async (input) => { + await expect(flag.parse(input, {} as any, flag as any)).rejects.toThrowError(/Expected an integer/) + }, + ) +}) diff --git a/packages/cli-kit/src/public/node/cli.ts b/packages/cli-kit/src/public/node/cli.ts index fd2836734f8..10a6a2eb006 100644 --- a/packages/cli-kit/src/public/node/cli.ts +++ b/packages/cli-kit/src/public/node/cli.ts @@ -153,6 +153,19 @@ export const jsonFlag = { }), } +/** + * Builds a `--port` flag that only accepts a valid port number. The flag parses its + * value as an integer and rejects anything that isn't a whole number between 1 and + * 65535, so commands fail with a clear message instead of crashing on an out-of-range + * port. The accepted range is appended to the description so it shows up in `--help`. + * @param options - Optional overrides for the flag's description, environment variable, and visibility. + * @returns An oclif integer flag constrained to the valid port range. + */ +export const portFlag = (options: {description?: string; env?: string; hidden?: boolean} = {}) => { + const description = [options.description, 'Must be between 1 and 65535.'].filter(Boolean).join(' ') + return Flags.integer({min: 1, max: 65535, ...options, description}) +} + /** * Clear the CLI cache, used to store some API responses and handle notifications status */ diff --git a/packages/cli-kit/src/public/node/tcp.test.ts b/packages/cli-kit/src/public/node/tcp.test.ts index 3a42698e4fc..6b40b85e455 100644 --- a/packages/cli-kit/src/public/node/tcp.test.ts +++ b/packages/cli-kit/src/public/node/tcp.test.ts @@ -90,4 +90,8 @@ describe('checkPortAvailability', () => { server.close() } }) + + test.each([13245574, 70000, -1, NaN])('resolves false for out-of-range/invalid port %s', async (port) => { + await expect(checkPortAvailability(port)).resolves.toBe(false) + }) }) diff --git a/packages/cli-kit/src/public/node/tcp.ts b/packages/cli-kit/src/public/node/tcp.ts index ed194b450b7..96e2ca9e9c8 100644 --- a/packages/cli-kit/src/public/node/tcp.ts +++ b/packages/cli-kit/src/public/node/tcp.ts @@ -49,9 +49,16 @@ export async function checkPortAvailability(portNumber: number): Promise resolve(false)) - server.listen(portNumber, 'localhost', () => { - server.close(() => resolve(true)) - }) + try { + server.listen(portNumber, 'localhost', () => { + server.close(() => resolve(true)) + }) + } catch (error) { + if (!(error instanceof RangeError)) { + throw error + } + resolve(false) + } }) } diff --git a/packages/cli/README.md b/packages/cli/README.md index d96a305ea8a..f7d6f3620d6 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -404,7 +404,8 @@ FLAGS --checkout-cart-url= [env: SHOPIFY_FLAG_CHECKOUT_CART_URL] Resource URL for checkout UI extension. Format: "/cart/{productVariantID}:{productQuantity}" --client-id= [env: SHOPIFY_FLAG_CLIENT_ID] The Client ID of your app. - --localhost-port= [env: SHOPIFY_FLAG_LOCALHOST_PORT] Port to use for localhost. + --localhost-port= [env: SHOPIFY_FLAG_LOCALHOST_PORT] Port to use for localhost. Must be between + 1 and 65535. --no-color [env: SHOPIFY_FLAG_NO_COLOR] Disable color output. --no-update [env: SHOPIFY_FLAG_NO_UPDATE] Uses the app URL from the toml file instead an autogenerated URL for dev. @@ -418,7 +419,7 @@ FLAGS --subscription-product-url= [env: SHOPIFY_FLAG_SUBSCRIPTION_PRODUCT_URL] Resource URL for subscription UI extension. Format: "/products/{productId}" --theme-app-extension-port= [env: SHOPIFY_FLAG_THEME_APP_EXTENSION_PORT] Local port of the theme app - extension development server. + extension development server. Must be between 1 and 65535. --tunnel-url= [env: SHOPIFY_FLAG_TUNNEL_URL] Use a custom tunnel, it must be running before executing dev. Format: "https://my-tunnel-url:port". --use-localhost [env: SHOPIFY_FLAG_USE_LOCALHOST] Service entry point will listen to @@ -2361,7 +2362,7 @@ FLAGS [env: SHOPIFY_FLAG_PATH] The path where you want to run the command. Defaults to the current working directory. --port= - [env: SHOPIFY_FLAG_PORT] Local port to serve theme preview from. + [env: SHOPIFY_FLAG_PORT] Local port to serve theme preview from. Must be between 1 and 65535. --store-password= [env: SHOPIFY_FLAG_STORE_PASSWORD] The password for storefronts with password protection. diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json index b73dd5a6473..eeacfe2b751 100644 --- a/packages/cli/oclif.manifest.json +++ b/packages/cli/oclif.manifest.json @@ -935,7 +935,7 @@ "type": "option" }, "graphiql-port": { - "description": "Local port of the GraphiQL development server.", + "description": "Local port of the GraphiQL development server. Must be between 1 and 65535.", "env": "SHOPIFY_FLAG_GRAPHIQL_PORT", "hasDynamicHelp": false, "hidden": true, @@ -944,7 +944,7 @@ "type": "option" }, "localhost-port": { - "description": "Port to use for localhost.", + "description": "Port to use for localhost. Must be between 1 and 65535.", "env": "SHOPIFY_FLAG_LOCALHOST_PORT", "hasDynamicHelp": false, "multiple": false, @@ -1028,7 +1028,7 @@ "type": "option" }, "theme-app-extension-port": { - "description": "Local port of the theme app extension development server.", + "description": "Local port of the theme app extension development server. Must be between 1 and 65535.", "env": "SHOPIFY_FLAG_THEME_APP_EXTENSION_PORT", "hasDynamicHelp": false, "multiple": false, @@ -6362,7 +6362,7 @@ "type": "boolean" }, "port": { - "description": "Local port to serve theme preview from.", + "description": "Local port to serve theme preview from. Must be between 1 and 65535.", "env": "SHOPIFY_FLAG_PORT", "hasDynamicHelp": false, "multiple": false, diff --git a/packages/theme/src/cli/commands/theme/dev.ts b/packages/theme/src/cli/commands/theme/dev.ts index c749ed07b60..b38af843f3b 100644 --- a/packages/theme/src/cli/commands/theme/dev.ts +++ b/packages/theme/src/cli/commands/theme/dev.ts @@ -6,7 +6,7 @@ import {findOrSelectTheme} from '../../utilities/theme-selector.js' import {metafieldsPull} from '../../services/metafields-pull.js' import {ensureLiveThemeConfirmed} from '../../utilities/theme-ui.js' import {Flags} from '@oclif/core' -import {globalFlags} from '@shopify/cli-kit/node/cli' +import {globalFlags, portFlag} from '@shopify/cli-kit/node/cli' import {Theme} from '@shopify/cli-kit/node/themes/types' import {recordEvent} from '@shopify/cli-kit/node/analytics' import {AdminSession} from '@shopify/cli-kit/node/session' @@ -77,7 +77,7 @@ You can run this command only in a directory that matches the [default Shopify t description: 'Synchronize Theme Editor updates in the local theme files.', env: 'SHOPIFY_FLAG_THEME_EDITOR_SYNC', }), - port: Flags.string({ + port: portFlag({ description: 'Local port to serve theme preview from.', env: 'SHOPIFY_FLAG_PORT', }), diff --git a/packages/theme/src/cli/services/dev.test.ts b/packages/theme/src/cli/services/dev.test.ts index 2a79bf50f16..3ab42255b3c 100644 --- a/packages/theme/src/cli/services/dev.test.ts +++ b/packages/theme/src/cli/services/dev.test.ts @@ -354,3 +354,75 @@ describe('dev() Ctrl-C analytics', () => { expect(reportAnalyticsEvent).toHaveBeenCalledTimes(1) }) }) + +describe('dev() port validation', () => { + const mockConfig = {} as unknown as Config + const adminSession = {storeFqdn: 'test.myshopify.com', token: 'x'} + + let exitSpy: MockInstance + let resolveBackgroundJob: () => void + + const baseOptions = { + adminSession, + commandConfig: mockConfig, + directory: '/tmp/theme', + store: 'test.myshopify.com', + open: false, + theme, + force: false, + 'theme-editor-sync': false, + 'live-reload': 'hot-reload' as const, + 'error-overlay': 'default' as const, + noDelete: false, + ignore: [], + only: [], + } + + beforeEach(() => { + vi.mocked(hasRequiredThemeDirectories).mockResolvedValue(true) + vi.mocked(isStorefrontPasswordProtected).mockResolvedValue(false) + vi.mocked(initializeDevServerSession).mockResolvedValue({ + storeFqdn: adminSession.storeFqdn, + token: adminSession.token, + } as any) + vi.mocked(getAvailableTCPPort).mockResolvedValue(9292) + vi.mocked(checkPortAvailability).mockResolvedValue(true) + + const backgroundJobPromise = new Promise((resolve) => { + resolveBackgroundJob = resolve + }) + vi.mocked(setupDevServer).mockReturnValue({ + serverStart: vi.fn().mockResolvedValue(undefined), + renderDevSetupProgress: vi.fn().mockResolvedValue(undefined), + backgroundJobPromise, + resolveBackgroundJob: resolveBackgroundJob!, + dispatchEvent: vi.fn(), + } as any) + + exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => undefined as never) + }) + + afterEach(() => { + vi.clearAllMocks() + exitSpy.mockRestore() + }) + + test('accepts a valid port and calls checkPortAvailability with it', async () => { + vi.mocked(checkPortAvailability).mockResolvedValue(true) + + const devPromise = dev({...baseOptions, port: 9293}) + + await new Promise((resolve) => setImmediate(resolve)) + + resolveBackgroundJob() + await devPromise + + expect(checkPortAvailability).toHaveBeenCalledWith(9293) + }) + + test('rejects a valid but unavailable port', async () => { + vi.mocked(checkPortAvailability).mockResolvedValue(false) + + await expect(dev({...baseOptions, port: 9293})).rejects.toThrowError(/is not available/) + }) +}) diff --git a/packages/theme/src/cli/services/dev.ts b/packages/theme/src/cli/services/dev.ts index 6f213e45b87..32434d6bea9 100644 --- a/packages/theme/src/cli/services/dev.ts +++ b/packages/theme/src/cli/services/dev.ts @@ -23,7 +23,7 @@ import {Config} from '@oclif/core' import readline from 'readline' const DEFAULT_HOST = '127.0.0.1' -const DEFAULT_PORT = '9292' +const DEFAULT_PORT = 9292 let hasReportedAnalyticsEvent = false @@ -37,7 +37,7 @@ interface DevOptions { open: boolean theme: Theme host?: string - port?: string + port?: number force: boolean 'theme-editor-sync': boolean 'live-reload': LiveReload @@ -93,14 +93,18 @@ export async function dev(options: DevOptions) { }) const host = options.host ?? DEFAULT_HOST - if (options.port && !(await checkPortAvailability(Number(options.port)))) { - throw new AbortError( - `Port ${options.port} is not available. Try a different port or remove the --port flag to use an available port.`, - ) + let port: number + if (options.port) { + if (!(await checkPortAvailability(options.port))) { + throw new AbortError( + `Port ${options.port} is not available. Try a different port or remove the --port flag to use an available port.`, + ) + } + port = options.port + } else { + port = await getAvailableTCPPort(DEFAULT_PORT) } - const port = options.port ?? String(await getAvailableTCPPort(Number(DEFAULT_PORT))) - const urls = { local: `http://${host}:${port}`, giftCard: `http://${host}:${port}/gift_cards/[store_id]/preview`, diff --git a/packages/theme/src/cli/utilities/theme-environment/hot-reload/server.test.ts b/packages/theme/src/cli/utilities/theme-environment/hot-reload/server.test.ts index 3d2905a369b..261e5541e87 100644 --- a/packages/theme/src/cli/utilities/theme-environment/hot-reload/server.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/hot-reload/server.test.ts @@ -555,7 +555,7 @@ function createTestContext(options?: {files?: [string, string][]}) { only: [], noDelete: true, host: '', - port: '', + port: 9292, liveReload: 'hot-reload', open: false, themeEditorSync: false, diff --git a/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts b/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts index efacdc852ea..beb524f3384 100644 --- a/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts @@ -29,7 +29,7 @@ describe('dev proxy', () => { const ctx = { session: {storeFqdn: 'my-store.myshopify.com', sessionCookies: {}}, - options: {host: 'localhost', port: '1337'}, + options: {host: 'localhost', port: 1337}, localThemeFileSystem: {files: new Map([['assets/file1', 'content']])}, localThemeExtensionFileSystem: {files: new Map([['assets/file-ext', 'content']])}, } as unknown as DevServerContext diff --git a/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts b/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts index f4ef8a89a9c..ff76c8b24af 100644 --- a/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts @@ -99,7 +99,7 @@ describe('setupDevServer', () => { only: ['templates/*.liquid'], noDelete: true, host: '127.0.0.1', - port: '9292', + port: 9292, liveReload: 'hot-reload', open: false, themeEditorSync: false, diff --git a/packages/theme/src/cli/utilities/theme-environment/types.ts b/packages/theme/src/cli/utilities/theme-environment/types.ts index 9b689c7daf8..d01377975ea 100644 --- a/packages/theme/src/cli/utilities/theme-environment/types.ts +++ b/packages/theme/src/cli/utilities/theme-environment/types.ts @@ -123,7 +123,7 @@ export interface DevServerContext { /** * Port to bind the development server to. */ - port: string + port: number /** * Mode for live reload behavior. Options: ['hot-reload', 'full-page', 'off'] diff --git a/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-server.ts b/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-server.ts index 0ff73ab674d..b305e32b5c5 100644 --- a/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-server.ts +++ b/packages/theme/src/cli/utilities/theme-ext-environment/theme-ext-server.ts @@ -91,7 +91,7 @@ async function contextDevServerContext( ignore: [], only: [], host, - port: port.toString(), + port, liveReload: 'hot-reload', open: false, errorOverlay: 'default',