diff --git a/.changeset/theme-extension-network-access-allowed-sources.md b/.changeset/theme-extension-network-access-allowed-sources.md new file mode 100644 index 00000000000..123061e9b23 --- /dev/null +++ b/.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`. diff --git a/packages/app/src/cli/models/app/loader.test.ts b/packages/app/src/cli/models/app/loader.test.ts index 1f97c353388..44471a40578 100644 --- a/packages/app/src/cli/models/app/loader.test.ts +++ b/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) diff --git a/packages/app/src/cli/models/extensions/schemas.ts b/packages/app/src/cli/models/extensions/schemas.ts index 4c32cf3eb7c..509228f4570 100644 --- a/packages/app/src/cli/models/extensions/schemas.ts +++ b/packages/app/src/cli/models/extensions/schemas.ts @@ -21,8 +21,11 @@ const IframeCapabilitySchema = zod.object({ sources: zod.array(zod.string()).optional(), }) -const CapabilitiesSchema = zod.object({ - network_access: zod.boolean().optional(), +export const CapabilitiesSchema = zod.object({ + // Theme app extensions declare `network_access` as a table that lists the origins they contact (see the theme + // specification). Every other extension declares a boolean, which is all this shared schema accepts. The wider + // type lets the theme specification reuse `BaseConfigType`. + network_access: zod.boolean().optional() as ZodSchemaType, block_progress: zod.boolean().optional(), api_access: zod.boolean().optional(), collect_buyer_consent: CollectBuyerConsentCapabilitySchema.optional(), diff --git a/packages/app/src/cli/models/extensions/specifications/theme.test.ts b/packages/app/src/cli/models/extensions/specifications/theme.test.ts new file mode 100644 index 00000000000..5023a10e1fe --- /dev/null +++ b/packages/app/src/cli/models/extensions/specifications/theme.test.ts @@ -0,0 +1,201 @@ +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) { + 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('ignores a boolean network_access, as before', () => { + const result = parse({network_access: true}) + + expect(result.state).toBe('ok') + expect(result.data?.capabilities?.network_access).toBeUndefined() + }) + + 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 neither a table nor a boolean', () => { + 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 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 network_access is a boolean', async () => { + const config = parse({network_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('still accept a boolean network_access', () => { + const result = UIExtensionSchema.safeParse({ + name: 'ui extension', + type: 'ui_extension', + extension_points: [], + capabilities: {network_access: true}, + }) + + expect(result.success).toBe(true) + expect(result.data?.capabilities?.network_access).toBe(true) + }) + + 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) + }) + }) +}) diff --git a/packages/app/src/cli/models/extensions/specifications/theme.ts b/packages/app/src/cli/models/extensions/specifications/theme.ts index 8e6d27d27e4..37a02c2a153 100644 --- a/packages/app/src/cli/models/extensions/specifications/theme.ts +++ b/packages/app/src/cli/models/extensions/specifications/theme.ts @@ -1,15 +1,91 @@ import {createExtensionSpecification} from '../specification.js' -import {BaseSchema} from '../schemas.js' +import {BaseSchema, CapabilitiesSchema} from '../schemas.js' import {themeExtensionFiles} from '../../../utilities/extensions/theme.js' import {ExtensionInstance} from '../extension-instance.js' import {fileSize} from '@shopify/cli-kit/node/fs' import {dirname, relativePath} from '@shopify/cli-kit/node/path' import {AbortError} from '@shopify/cli-kit/node/error' import {outputContent, outputToken} from '@shopify/cli-kit/node/output' +import {zod} from '@shopify/cli-kit/node/schema' + +export const MAX_ALLOWED_SOURCES = 20 +const MAX_HOST_LENGTH = 253 +const DEFAULT_HTTPS_PORT = 443 +const MAX_PORT = 65535 +const ALLOWED_SOURCE_PATTERN = /^https:\/\/(\*\.)?([a-z0-9.-]+)(?::(\d{1,5}))?$/ +const HOST_LABEL_PATTERN = /^[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?$/ + +/** + * Normalizes an `allowed_sources` entry to `https://[*.]host[:port]`. + * + * Entries are trimmed and lowercased, a single trailing slash is dropped, and the default `:443` port is removed. + * The host needs at least two labels and can't be an IP address. Only a leading `*.` wildcard is allowed. + * + * @param source - The entry as written in `shopify.extension.toml`. + * @returns The normalized origin, or `undefined` if the entry isn't a valid HTTPS origin. + */ +export function normalizeAllowedSource(source: string): string | undefined { + const value = source.trim().toLowerCase().replace(/\/$/, '') + const match = ALLOWED_SOURCE_PATTERN.exec(value) + if (!match) return undefined + + const [, wildcard = '', host = '', port] = match + if (host.length > MAX_HOST_LENGTH) return undefined + + const labels = host.split('.') + if (labels.length < 2 || !labels.every((label) => HOST_LABEL_PATTERN.test(label))) return undefined + // A numeric last label means an IP address (or a TLD that doesn't exist). + if (/^\d+$/.test(labels[labels.length - 1]!)) return undefined + + let portSuffix = '' + if (port !== undefined) { + const portNumber = Number(port) + if (portNumber < 1 || portNumber > MAX_PORT) return undefined + if (portNumber !== DEFAULT_HTTPS_PORT) portSuffix = `:${portNumber}` + } + + return `https://${wildcard}${host}${portSuffix}` +} + +const AllowedSourcesSchema = zod + .array(zod.string(), { + required_error: 'allowed_sources is required. Use an empty list if your extension contacts no other origins.', + invalid_type_error: 'allowed_sources must be a list of HTTPS origins, like ["https://api.example.com"]', + }) + .superRefine((sources, ctx) => { + const invalid = sources.filter((source) => normalizeAllowedSource(source) === undefined) + if (invalid.length > 0) { + ctx.addIssue({ + code: zod.ZodIssueCode.custom, + message: `Invalid allowed_sources: ${invalid.map((source) => JSON.stringify(source)).join(', ')}. Use HTTPS origins with no path, query or fragment, like "https://api.example.com" or "https://*.cdn.example.com".`, + }) + } + }) + .transform((sources) => [...new Set(sources.map((source) => normalizeAllowedSource(source)!))]) + .refine((sources) => sources.length <= MAX_ALLOWED_SOURCES, { + message: `allowed_sources can list up to ${MAX_ALLOWED_SOURCES} origins`, + }) + +const ThemeNetworkAccessSchema = zod.object( + {allowed_sources: AllowedSourcesSchema}, + {invalid_type_error: 'network_access must be a table with an allowed_sources list for theme app extensions'}, +) + +const ThemeCapabilitiesSchema = CapabilitiesSchema.extend({ + // A boolean `network_access` (used by UI extensions) never had an effect on theme app extensions. Keep ignoring it. + network_access: zod.preprocess( + (value) => (typeof value === 'boolean' ? undefined : value), + ThemeNetworkAccessSchema.optional(), + ), +}) + +const ThemeSchema = BaseSchema.extend({ + capabilities: ThemeCapabilitiesSchema.optional(), +}) const themeSpec = createExtensionSpecification({ identifier: 'theme', - schema: BaseSchema, + schema: ThemeSchema, partnersWebIdentifier: 'theme_app_extension', graphQLType: 'theme_app_extension', clientSteps: [ @@ -24,8 +100,15 @@ const themeSpec = createExtensionSpecification({ appModuleFeatures: (_) => { return ['theme'] }, - deployConfig: async () => { - return {theme_extension: {files: {}}} + deployConfig: async (config) => { + const networkAccess = config.capabilities?.network_access + // Only send capabilities when they're declared, so existing payloads stay the same. + if (typeof networkAccess !== 'object') return {theme_extension: {files: {}}} + + return { + theme_extension: {files: {}}, + capabilities: {network_access: {allowed_sources: networkAccess.allowed_sources}}, + } }, preDeployValidation: async (extension) => { return validateThemeExtension(extension) diff --git a/packages/app/src/cli/services/dev/extension/payload.ts b/packages/app/src/cli/services/dev/extension/payload.ts index aed224c70a1..8afeeecbd09 100644 --- a/packages/app/src/cli/services/dev/extension/payload.ts +++ b/packages/app/src/cli/services/dev/extension/payload.ts @@ -98,7 +98,7 @@ export async function getUIExtensionPayload( }, capabilities: { blockProgress: extension.configuration.capabilities?.block_progress ?? false, - networkAccess: extension.configuration.capabilities?.network_access ?? false, + networkAccess: extension.configuration.capabilities?.network_access === true, apiAccess: extension.configuration.capabilities?.api_access ?? false, collectBuyerConsent: { smsMarketing: extension.configuration.capabilities?.collect_buyer_consent?.sms_marketing ?? false,