Let theme app extensions declare the network origins they contact

.changeset/theme-extension-network-access-allowed-sources.md

@@ -0,0 +1,5 @@
+---
+'@shopify/app': minor
+---
+
+Let theme app extensions declare the network origins they contact with `[extensions.capabilities.network_access] allowed_sources` in `shopify.extension.toml`.

packages/app/src/cli/models/app/loader.test.ts

@@ -488,6 +488,31 @@ describe('load', () => {
     expect(app.errors.isEmpty()).toBe(true)
   })
 
+  test('loads network_access allowed_sources for theme extensions', async () => {
+    // Given
+    await writeConfig(appConfiguration)
+    const blockConfiguration = `
+      name = "my_extension"
+      type = "theme"
+
+      [capabilities.network_access]
+      allowed_sources = ["https://API.example.com/", "https://*.cdn.example.com"]
+      `
+    await writeBlockConfig({blockConfiguration, name: 'my-extension'})
+    await writeFile(joinPath(blockPath('my-extension'), 'index.js'), '')
+
+    // When
+    const app = await loadTestingApp()
+
+    // Then
+    expect(app.errors.isEmpty()).toBe(true)
+    const extension = getRealExtensions(app)[0]!
+    await expect(extension.deployConfig({apiKey: 'api-key', appConfiguration: app.configuration})).resolves.toEqual({
+      theme_extension: {files: {}},
+      capabilities: {network_access: {allowed_sources: ['https://api.example.com', 'https://*.cdn.example.com']}},
+    })
+  })
+
   test('reports error for duplicated handles when ignoreUnknownExtensions is true', async () => {
     // Given
     await writeConfig(appConfiguration)

packages/app/src/cli/models/extensions/extension-instance.ts

@@ -1,4 +1,4 @@
-import {BaseConfigType, MAX_EXTENSION_HANDLE_LENGTH, MAX_UID_LENGTH} from './schemas.js'
+import {BaseConfigType, BaseExtensionConfigType, MAX_EXTENSION_HANDLE_LENGTH, MAX_UID_LENGTH} from './schemas.js'
 import {FunctionConfigType} from './specifications/function.js'
 import {
   DevSessionUpdateContext,
@@ -58,7 +58,7 @@ export const DEFAULT_DEV_SESSION_UPDATE_MESSAGE = 'Configuration accepted'
  *
  * This class holds the public interface to interact with extensions
  */
-export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfigType> {
+export class ExtensionInstance<TConfiguration extends BaseExtensionConfigType = BaseConfigType> {
   entrySourceFilePath: string
   devUUID: string
   localIdentifier: string

packages/app/src/cli/models/extensions/schemas.ts

@@ -154,5 +154,7 @@ export const UnifiedSchema = zod.object({
 
 export type NewExtensionPointSchemaType = zod.infer<typeof NewExtensionPointSchema>
 
-// Base config type that all config schemas must extend.
 export type BaseConfigType = zod.infer<typeof BaseSchema>
+
+// Base config type that all config schemas must extend.
+export type BaseExtensionConfigType = Omit<BaseConfigType, 'capabilities'>

packages/app/src/cli/models/extensions/specification.ts

@@ -1,4 +1,4 @@
-import {ZodSchemaType, BaseConfigType, BaseSchema} from './schemas.js'
+import {ZodSchemaType, BaseConfigType, BaseExtensionConfigType, BaseSchema} from './schemas.js'
 import {ExtensionInstance} from './extension-instance.js'
 import {patchAppRelativeUrls} from './specifications/validation/app_relative_urls.js'
 import {blocks} from '../../constants.js'
@@ -81,7 +81,7 @@ export interface DevSessionUpdateContext {
 /**
  * Extension specification with all the needed properties and methods to load an extension.
  */
-export interface ExtensionSpecification<TConfiguration extends BaseConfigType = BaseConfigType> {
+export interface ExtensionSpecification<TConfiguration extends BaseExtensionConfigType = BaseConfigType> {
   identifier: string
   externalIdentifier: string
   externalName: string
@@ -179,7 +179,7 @@ export interface DevSessionWatchConfig {
 /**
  * Extension specification, explicitly marked as having taken remote configuration values into account.
  */
-export type RemoteAwareExtensionSpecification<TConfiguration extends BaseConfigType = BaseConfigType> =
+export type RemoteAwareExtensionSpecification<TConfiguration extends BaseExtensionConfigType = BaseConfigType> =
   ExtensionSpecification<TConfiguration> & {
     loadedRemoteSpecs: true
   }
@@ -202,7 +202,7 @@ type ForbiddenFields =
 /**
  * Partial ExtensionSpec type used when creating a new ExtensionSpec, the only mandatory field is the identifier
  */
-interface CreateExtensionSpecType<TConfiguration extends BaseConfigType = BaseConfigType> extends Partial<
+interface CreateExtensionSpecType<TConfiguration extends BaseExtensionConfigType = BaseConfigType> extends Partial<
   Omit<ExtensionSpecification<TConfiguration>, ForbiddenFields>
 > {
   identifier: string
@@ -229,9 +229,10 @@ interface CreateExtensionSpecType<TConfiguration extends BaseConfigType = BaseCo
  * hasExtensionPointTarget?: (configuration: TConfiguration, target: string) => boolean // function to determine if the extension has a given extension point target
  * ```
  */
-export function createExtensionSpecification<TConfiguration extends BaseConfigType = BaseConfigType>(
+export function createExtensionSpecification<TConfiguration extends BaseExtensionConfigType = BaseConfigType>(
   spec: CreateExtensionSpecType<TConfiguration>,
 ): ExtensionSpecification<TConfiguration> {
+  const baseSchema: ZodSchemaType<BaseExtensionConfigType> = BaseSchema
   const defaults = {
     // these two fields are going to be overridden by the extension specification API response,
     // but we need them to have a default value for tests
@@ -240,7 +241,7 @@ export function createExtensionSpecification<TConfiguration extends BaseConfigTy
     externalName: capitalize(spec.identifier.replace(/_/g, ' ')),
     surface: 'test-surface',
     partnersWebIdentifier: spec.identifier,
-    schema: BaseSchema as ZodSchemaType<TConfiguration>,
+    schema: baseSchema as ZodSchemaType<TConfiguration>,
     registrationLimit: blocks.extensions.defaultRegistrationLimit,
     transform: spec.transformLocalToRemote,
     reverseTransform: spec.transformRemoteToLocal,
@@ -292,7 +293,9 @@ export function createExtensionSpecification<TConfiguration extends BaseConfigTy
  *   - App config extensions have default transform and reverseTransform functions
 
  */
-export function createConfigExtensionSpecification<TConfiguration extends BaseConfigType = BaseConfigType>(spec: {
+export function createConfigExtensionSpecification<
+  TConfiguration extends BaseExtensionConfigType = BaseConfigType,
+>(spec: {
   identifier: string
   schema: ZodSchemaType<TConfiguration>
   clientSteps?: ClientSteps
@@ -325,7 +328,7 @@ export function createConfigExtensionSpecification<TConfiguration extends BaseCo
   })
 }
 
-export function createContractBasedModuleSpecification<TConfiguration extends BaseConfigType = BaseConfigType>(
+export function createContractBasedModuleSpecification<TConfiguration extends BaseExtensionConfigType = BaseConfigType>(
   spec: Pick<
     CreateExtensionSpecType<TConfiguration>,
     | 'identifier'

packages/app/src/cli/models/extensions/specifications/theme.test.ts

@@ -0,0 +1,217 @@
+import themeSpec, {MAX_ALLOWED_SOURCES, normalizeAllowedSource} from './theme.js'
+import {UIExtensionSchema} from './ui_extension.js'
+import {describe, expect, test} from 'vitest'
+
+function parse(capabilities?: object) {
+  return themeSpec.parseConfigurationObject({
+    name: 'theme extension',
+    type: 'theme',
+    ...(capabilities === undefined ? {} : {capabilities}),
+  })
+}
+
+function parseSources(allowedSources: unknown) {
+  return parse({network_access: {allowed_sources: allowedSources}})
+}
+
+function errorMessages(result: ReturnType<typeof parse>) {
+  return result.state === 'error' ? result.errors.map((error) => error.message) : []
+}
+
+describe('theme', () => {
+  describe('capabilities.network_access.allowed_sources', () => {
+    test('accepts HTTPS origins and normalizes them', () => {
+      const result = parseSources([
+        'https://api.example.com',
+        ' HTTPS://CDN.Example.com/ ',
+        'https://*.cdn.example.com',
+        'https://api.example.com:443',
+        'https://api.example.com:8443',
+      ])
+
+      expect(result.state).toBe('ok')
+      expect(result.data?.capabilities?.network_access).toEqual({
+        allowed_sources: [
+          'https://api.example.com',
+          'https://cdn.example.com',
+          'https://*.cdn.example.com',
+          'https://api.example.com:8443',
+        ],
+      })
+    })
+
+    test('accepts an empty list', () => {
+      const result = parseSources([])
+
+      expect(result.state).toBe('ok')
+      expect(result.data?.capabilities?.network_access).toEqual({allowed_sources: []})
+    })
+
+    test('leaves network_access undefined when it is omitted', () => {
+      expect(parse().data?.capabilities).toBeUndefined()
+      expect(parse({}).data?.capabilities?.network_access).toBeUndefined()
+    })
+
+    test.each([true, false])('rejects a boolean network_access of %s', (networkAccess) => {
+      const result = parse({network_access: networkAccess})
+
+      expect(result.state).toBe('error')
+      expect(errorMessages(result)).toEqual([
+        'network_access must be a table with an allowed_sources list for theme app extensions',
+      ])
+    })
+
+    test('ignores capabilities that do not apply to theme extensions', () => {
+      const result = parse({network_access: {allowed_sources: []}, api_access: true, block_progress: true})
+
+      expect(result.state).toBe('ok')
+      expect(result.data?.capabilities).toEqual({network_access: {allowed_sources: []}})
+    })
+
+    test('requires allowed_sources when the network_access table is declared', () => {
+      const result = parse({network_access: {}})
+
+      expect(result.state).toBe('error')
+      expect(errorMessages(result)).toEqual([
+        'allowed_sources is required. Use an empty list if your extension contacts no other origins.',
+      ])
+    })
+
+    test('rejects a value that is not a list', () => {
+      const result = parseSources('https://api.example.com')
+
+      expect(result.state).toBe('error')
+      expect(errorMessages(result)).toEqual([
+        'allowed_sources must be a list of HTTPS origins, like ["https://api.example.com"]',
+      ])
+    })
+
+    test('rejects a network_access value that is not a table', () => {
+      const result = parse({network_access: 'https://api.example.com'})
+
+      expect(result.state).toBe('error')
+      expect(errorMessages(result)).toEqual([
+        'network_access must be a table with an allowed_sources list for theme app extensions',
+      ])
+    })
+
+    test.each([
+      ['an http origin', 'http://api.example.com'],
+      ['a missing scheme', 'api.example.com'],
+      ['a path', 'https://api.example.com/v1'],
+      ['a query', 'https://api.example.com?a=b'],
+      ['a fragment', 'https://api.example.com#a'],
+      ['credentials', 'https://user@api.example.com'],
+      ['a bare wildcard', '*'],
+      ['a wildcard on a top-level domain', 'https://*.com'],
+      ['a wildcard on a single label', 'https://*.example'],
+      ['a wildcard that is not the leading label', 'https://api.*.example.com'],
+      ['a single-label host', 'https://localhost'],
+      ['an IP address', 'https://127.0.0.1'],
+      ['an IP address with a hexadecimal final label', 'https://127.0.0x1'],
+      ['an IP address in hexadecimal notation', 'https://0x7f.0x1'],
+      ['a link-local IP address in hexadecimal notation', 'https://0xa9.0xfe.0xa9.0xfe'],
+      ['a wildcard on an IP address', 'https://*.127.0.0.1'],
+      ['a wildcard on an IP address in hexadecimal notation', 'https://*.0x7f.0x1'],
+      ['a host that cannot be parsed as a URL', 'https://example.0xabc'],
+      ['a wildcard on a host that cannot be parsed as a URL', 'https://*.example.0xabc'],
+      ['an invalid label', 'https://-api.example.com'],
+      ['an empty label', 'https://api..example.com'],
+      ['a trailing dot', 'https://api.example.com./'],
+      ['a newline', 'https://api.example.com\nhttps://other.example.com'],
+      ['port 0', 'https://api.example.com:0'],
+      ['a port above 65535', 'https://api.example.com:65536'],
+      ['a CSP keyword', "'self'"],
+      ['a CSP injection', 'https://api.example.com; script-src *'],
+    ])('rejects %s', (_description, source) => {
+      const result = parseSources(['https://ok.example.com', source])
+
+      expect(result.state).toBe('error')
+      expect(errorMessages(result)).toEqual([
+        `Invalid allowed_sources: ${JSON.stringify(
+          source,
+        )}. Use HTTPS origins with no path, query or fragment, like "https://api.example.com" or "https://*.cdn.example.com".`,
+      ])
+    })
+
+    test(`accepts up to ${MAX_ALLOWED_SOURCES} origins after removing duplicates`, () => {
+      const sources = Array.from({length: MAX_ALLOWED_SOURCES}, (_, index) => `https://api${index}.example.com`)
+
+      expect(parseSources([...sources, 'HTTPS://API0.EXAMPLE.COM']).state).toBe('ok')
+    })
+
+    test(`rejects more than ${MAX_ALLOWED_SOURCES} origins`, () => {
+      const sources = Array.from({length: MAX_ALLOWED_SOURCES + 1}, (_, index) => `https://api${index}.example.com`)
+      const result = parseSources(sources)
+
+      expect(result.state).toBe('error')
+      expect(errorMessages(result)).toEqual([`allowed_sources can list up to ${MAX_ALLOWED_SOURCES} origins`])
+    })
+  })
+
+  describe('normalizeAllowedSource', () => {
+    test('rejects a host longer than 253 characters', () => {
+      const host = `${'a'.repeat(60)}.`.repeat(5).concat('com')
+
+      expect(host.length).toBeGreaterThan(253)
+      expect(normalizeAllowedSource(`https://${host}`)).toBeUndefined()
+    })
+  })
+
+  describe('deployConfig', () => {
+    test('sends only the theme extension files when network_access is omitted', async () => {
+      const config = parse().data!
+
+      await expect(themeSpec.deployConfig?.(config, '', '', undefined)).resolves.toEqual({theme_extension: {files: {}}})
+    })
+
+    test('sends only the theme extension files when other capabilities are declared', async () => {
+      const config = parse({api_access: true, block_progress: true}).data!
+
+      await expect(themeSpec.deployConfig?.(config, '', '', undefined)).resolves.toEqual({theme_extension: {files: {}}})
+    })
+
+    test('sends the normalized allowed sources when they are declared', async () => {
+      const config = parseSources(['https://API.example.com/', 'https://*.cdn.example.com']).data!
+
+      await expect(themeSpec.deployConfig?.(config, '', '', undefined)).resolves.toEqual({
+        theme_extension: {files: {}},
+        capabilities: {network_access: {allowed_sources: ['https://api.example.com', 'https://*.cdn.example.com']}},
+      })
+    })
+
+    test('sends an empty list when allowed_sources is empty', async () => {
+      const config = parseSources([]).data!
+
+      await expect(themeSpec.deployConfig?.(config, '', '', undefined)).resolves.toEqual({
+        theme_extension: {files: {}},
+        capabilities: {network_access: {allowed_sources: []}},
+      })
+    })
+  })
+
+  describe('UI extensions', () => {
+    test.each([true, false])('still accept a boolean network_access of %s', (networkAccess) => {
+      const result = UIExtensionSchema.safeParse({
+        name: 'ui extension',
+        type: 'ui_extension',
+        extension_points: [],
+        capabilities: {network_access: networkAccess},
+      })
+
+      expect(result.success).toBe(true)
+      expect(result.data?.capabilities?.network_access).toBe(networkAccess)
+    })
+
+    test('reject a network_access table', () => {
+      const result = UIExtensionSchema.safeParse({
+        name: 'ui extension',
+        type: 'ui_extension',
+        extension_points: [],
+        capabilities: {network_access: {allowed_sources: ['https://api.example.com']}},
+      })
+
+      expect(result.success).toBe(false)
+    })
+  })
+})