Warn when a theme app extension contacts origins it doesn't declare (prototype)

.changeset/theme-extension-undeclared-origins.md

@@ -0,0 +1,5 @@
+---
+'@shopify/app': minor
+---
+
+Warn during build and deploy when a theme app extension loads from or sends requests to origins that its `[extensions.capabilities.network_access] allowed_sources` doesn't declare, and add the `THEME_EXTENSION_UNDECLARED_EGRESS` app security check.

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

@@ -108,6 +108,8 @@ const themeSpec = createExtensionSpecification({
     }
   },
   preDeployValidation: async (extension) => {
+    // Undeclared network origins are only a build warning for now (see build-theme-step.ts). Once storefronts enforce
+    // allowed_sources, deploys can fail here using the same detector (utilities/extensions/theme-egress.ts).
     return validateThemeExtension(extension.directory)
   },
 })

packages/app/src/cli/services/app-security-engine/checks/THEME_EXTENSION_UNDECLARED_EGRESS.md

@@ -0,0 +1,18 @@
+---
+id: THEME_EXTENSION_UNDECLARED_EGRESS
+version: 1
+severity: medium
+precedence: union
+---
+
+# Theme Extension Undeclared Egress
+
+Theme app extensions declare the origins their storefront code contacts in `shopify.extension.toml`, under `[capabilities.network_access] allowed_sources` (`[extensions.capabilities.network_access]` in the `[[extensions]]` format). Entries are HTTPS origins such as `https://api.example.com`, with an optional port. A leading `*.` covers one or more subdomain labels but not the bare domain. Requests to undeclared origins may be reported now and blocked on storefronts later.
+
+Inspect only theme app extensions (`type = "theme"`). The deterministic rule already reports literal URLs in Liquid and HTML (`<script src>`, fetching `<link href>`, `<iframe src>`, `<img src>`, `<source src>`, the `script_tag` and `stylesheet_tag` filters, inline `<script>` and `<style>`), in JavaScript assets (`fetch`, `XMLHttpRequest#open`, `WebSocket`, `EventSource`, `navigator.sendBeacon`, dynamic `import()`, `new URL`), and in CSS assets (`url()`, `@import`). Focus on what it can't resolve:
+
+- URLs built at runtime (template literals, concatenation, variables, Liquid output) whose origin is nonetheless fixed by the code, such as a constant base URL defined elsewhere in the extension.
+- Liquid files the parser couldn't read.
+- Sinks it doesn't scan, such as `srcset`, `<video>`/`<audio>` `src`, `style` attributes, `Worker` and `importScripts`, and requests made by third-party libraries bundled in the extension's assets.
+
+Report a finding only when the code establishes a concrete external origin that the extension's `allowed_sources` doesn't cover, or any `http:`/`ws:` origin, which can't be declared. Cite the line that makes the request. If the extension doesn't declare `allowed_sources`, report one finding on its `shopify.extension.toml` that lists the origins. Relative URLs, URLs from Liquid filters such as `asset_url`, the shop's own routes, and Shopify-owned hosts (`cdn.shopify.com`, `shopify.com`, `*.myshopify.com`) are not findings. A URL that a merchant configures in a block setting is not a finding, because its origin isn't known until runtime. Navigation (`<a href>`, `<form action>`, `location` changes, `window.open`) is out of scope.

packages/app/src/cli/services/app-security-engine/rules/catalog.ts

@@ -59,6 +59,17 @@ export const RULE_CATALOG: RuleCatalogEntry[] = [
     requires: 'theme_app_extension',
     docsUrl: `${CHECK_DOCS_BASE_URL}/liquid-unsafe-render`,
   },
+  {
+    id: 'THEME_EXTENSION_UNDECLARED_EGRESS',
+    title: 'Theme app extension contacts an undeclared origin',
+    severity: 'medium',
+    points: -10,
+    description:
+      'Detects literal origins that theme app extensions load from or send requests to without declaring them in network_access allowed_sources.',
+    fix: 'Declare the HTTPS origins the extension needs in capabilities.network_access.allowed_sources, or remove the requests.',
+    requires: 'theme_app_extension',
+    docsUrl: `${CHECK_DOCS_BASE_URL}/theme-extension-undeclared-egress`,
+  },
   {
     id: 'INSECURE_WEBHOOK_URL',
     title: 'Configured callback URL is not HTTPS',

packages/app/src/cli/services/app-security-engine/rules/theme-egress-rules.ts

@@ -0,0 +1,92 @@
+import {
+  allowedSourceFor,
+  declaredAllowedSources,
+  findOutboundOrigins,
+  type OutboundFinding,
+} from '../../../utilities/extensions/theme-egress.js'
+import {decodeToml} from '@shopify/cli-kit/node/toml/codec'
+import type {Issue} from '../types.js'
+import type {RunnerResult} from '../scanners/types.js'
+import type {ExtensionInfo, ScanContext, SourceFile} from './types.js'
+
+const CHECK_ID = 'THEME_EXTENSION_UNDECLARED_EGRESS'
+export const THEME_EGRESS_FILE_EXTENSIONS = ['.liquid', '.html', '.js', '.mjs', '.css']
+const ALLOWED_SOURCES_TABLE = 'capabilities.network_access.allowed_sources'
+const LISTED_ORIGINS = 3
+
+/**
+ * Finds origins that theme app extensions load from or send requests to without declaring them in
+ * `allowed_sources`. With the table declared, each undeclared origin is a finding where it's used. Without it, each
+ * extension that contacts external origins gets one finding on its configuration. Navigation targets and URLs built
+ * at runtime aren't findings.
+ */
+export function scanThemeExtensionUndeclaredEgress(context: ScanContext): RunnerResult {
+  const issues: Issue[] = []
+  const parserFailures: string[] = []
+  for (const extension of context.extensions.filter((candidate) => candidate.type === 'theme')) {
+    const files = extension.files.flatMap((file) =>
+      file.content !== undefined && THEME_EGRESS_FILE_EXTENSIONS.includes(file.ext)
+        ? [{path: file.path, content: file.content}]
+        : [],
+    )
+    const allowedSources = declaredAllowedSources(extensionCapabilities(extension))
+    const result = findOutboundOrigins(files, allowedSources)
+    parserFailures.push(...result.parserFailures)
+    const undeclared = result.findings.filter((finding) => finding.kind === 'egress' && finding.status === 'undeclared')
+    if (allowedSources) issues.push(...undeclared.map((finding) => undeclaredOriginIssue(finding, extension.files)))
+    else if (undeclared.length > 0) issues.push(undeclaredTableIssue(extension, undeclared))
+  }
+  return {
+    issues,
+    ...(parserFailures.length > 0 ? {unresolvedReason: `Liquid parser failed for: ${parserFailures.join(', ')}`} : {}),
+  }
+}
+
+function extensionCapabilities(extension: ExtensionInfo): unknown {
+  if (extension.content === undefined) return undefined
+  // findExtensions already parsed this TOML successfully.
+  return (decodeToml(extension.content) as {capabilities?: unknown}).capabilities
+}
+
+function undeclaredOriginIssue(finding: OutboundFinding, files: SourceFile[]): Issue {
+  const origin = finding.origin!
+  const content = files.find((file) => file.path === finding.file)?.content ?? ''
+  return {
+    id: CHECK_ID,
+    pattern_id: finding.insecure ? 'insecure-origin' : 'undeclared-origin',
+    severity: 'medium',
+    points: -10,
+    title: 'Theme app extension contacts an undeclared origin',
+    message: finding.insecure
+      ? `${finding.sink} contacts ${origin} over an insecure scheme. ${ALLOWED_SOURCES_TABLE} accepts only HTTPS origins, so this request can't be declared.`
+      : `${finding.sink} contacts ${origin}, which isn't in the extension's ${ALLOWED_SOURCES_TABLE}.`,
+    location: {file: finding.file, line: finding.line, column: finding.column},
+    snippet: content.split('\n')[finding.line - 1]?.trim(),
+    fix: {
+      automated: false,
+      description: finding.insecure
+        ? 'Load the resource over HTTPS and declare its origin, or remove the request.'
+        : `Add ${allowedSourceFor(origin) ?? origin} to ${ALLOWED_SOURCES_TABLE} in shopify.extension.toml, or remove the request.`,
+    },
+  }
+}
+
+function undeclaredTableIssue(extension: ExtensionInfo, findings: OutboundFinding[]): Issue {
+  const origins = [...new Set(findings.map((finding) => finding.origin!))]
+  const listed = origins.slice(0, LISTED_ORIGINS).join(', ')
+  const more = origins.length > LISTED_ORIGINS ? ` and ${origins.length - LISTED_ORIGINS} more` : ''
+  const count = origins.length === 1 ? '1 external origin' : `${origins.length} external origins`
+  return {
+    id: CHECK_ID,
+    pattern_id: 'allowed-sources-not-declared',
+    severity: 'medium',
+    points: -10,
+    title: 'Theme app extension contacts an undeclared origin',
+    message: `The theme app extension contacts ${count} (${listed}${more}) but doesn't declare ${ALLOWED_SOURCES_TABLE}.`,
+    location: {file: extension.path, line: 1, column: 1},
+    fix: {
+      automated: false,
+      description: `Declare the HTTPS origins the extension needs in ${ALLOWED_SOURCES_TABLE} in shopify.extension.toml.`,
+    },
+  }
+}

packages/app/src/cli/services/app-security-engine/scanners/discover.ts

@@ -408,6 +408,7 @@ export function findExtensions(appRoot: string, repositoryFiles: ReadonlyArray<s
       const raw = decodeToml(content) as Record<string, unknown>
       const type = raw.type as string
       const files = findAppSourceFiles(appRoot, sourcePathsByDirectory.get(dirname(tomlPath)) ?? [])
+      if (type === 'theme') files.push(...findThemeOnlySourceFiles(appRoot, repositoryFiles, dirname(tomlPath)))
       return [{path: tomlPath, type, content, files}]
       // Invalid repository TOML is a coverage gap, not a scanner crash.
       // eslint-disable-next-line no-catch-all/no-catch-all
@@ -724,7 +725,37 @@ export function findSourceCandidates(repositoryFiles: ReadonlyArray<string>): So
 
 /** Find and read only source languages supported by non-secret deterministic scanners. */
 export function findAppSourceFiles(appRoot: string, repositoryFiles: ReadonlyArray<string>): SourceFile[] {
-  return repositoryFiles.filter(hasSupportedSourceExtension).map((path) => {
+  return readSourceFiles(appRoot, repositoryFiles.filter(hasSupportedSourceExtension))
+}
+
+// Theme app extensions can contact other origins from CSS (`url()` and `@import`), so their CSS is read too. It isn't
+// app source, so it stays out of the language inventory and the app source checks.
+const THEME_ONLY_SOURCE_EXTENSIONS = new Set(['.css'])
+
+function findThemeOnlySourceFiles(
+  appRoot: string,
+  repositoryFiles: ReadonlyArray<string>,
+  extensionDirectory: string,
+): SourceFile[] {
+  return readSourceFiles(
+    appRoot,
+    repositoryFiles.filter(
+      (path) => THEME_ONLY_SOURCE_EXTENSIONS.has(extname(path)) && isInDirectory(path, extensionDirectory),
+    ),
+  )
+}
+
+function isInDirectory(path: string, directory: string): boolean {
+  let ancestor = dirname(path)
+  while (ancestor !== '.') {
+    if (ancestor === directory) return true
+    ancestor = dirname(ancestor)
+  }
+  return directory === '.'
+}
+
+function readSourceFiles(appRoot: string, paths: ReadonlyArray<string>): SourceFile[] {
+  return paths.map((path) => {
     const absolutePath = joinPath(appRoot, path)
     const result = readRepositoryFile(absolutePath)
     return {

packages/app/src/cli/services/app-security-engine/scanners/index.ts

@@ -28,6 +28,7 @@ import {
   scanUnsafeInnerHTML,
 } from '../rules/js-rules.js'
 import {scanLiquidSecurity} from '../rules/liquid-rules.js'
+import {scanThemeExtensionUndeclaredEgress, THEME_EGRESS_FILE_EXTENSIONS} from '../rules/theme-egress-rules.js'
 import {redactText, scanCommittedSecrets} from '../rules/secret-rules.js'
 import {scanDeprecatedScriptTagApi} from '../rules/shopify-rules.js'
 import {missingComplianceWebhooks, scanEolApiVersions} from '../rules/compliance-rules.js'
@@ -168,6 +169,16 @@ const DETERMINISTIC_CHECK_DEFINITIONS: ReadonlyArray<DeterministicCheckDefinitio
     extensions: ['.liquid', '.html'],
     runner: (context) => liquidRunner(context, 'LIQUID_UNSAFE_RENDER'),
   },
+  {
+    id: 'THEME_EXTENSION_UNDECLARED_EGRESS',
+    version: 1,
+    lifecycle: 'active',
+    analysisMode: 'regex',
+    target: 'theme',
+    requires: 'theme_app_extension',
+    extensions: THEME_EGRESS_FILE_EXTENSIONS,
+    runner: (context) => scanThemeExtensionUndeclaredEgress(context),
+  },
   {
     ...jsCheck('UNSAFE_INNERHTML', unsafeInnerHtmlRunner, 'source_and_theme', 4),
     analysisMode: 'regex',

packages/app/src/cli/services/app-security-engine/tests/catalog.test.ts

@@ -38,6 +38,7 @@ const DOCS_PAGE_SLUGS = [
   'ssrf-request-forgery',
   'static-frame-ancestors',
   'text-setting-html-smuggling',
+  'theme-extension-undeclared-egress',
   'theme-extension-xss',
   'unauthenticated-endpoint',
   'unsafe-innerhtml',

packages/app/src/cli/services/app-security-engine/tests/checks.test.ts

@@ -38,6 +38,7 @@ const EXPECTED_CHECK_IDS = [
   'SSRF_REQUEST_FORGERY',
   'STATIC_FRAME_ANCESTORS',
   'TEXT_SETTING_HTML_SMUGGLING',
+  'THEME_EXTENSION_UNDECLARED_EGRESS',
   'THEME_EXTENSION_XSS',
   'UNAUTHENTICATED_ENDPOINT',
   'UNSAFE_INNERHTML',

packages/app/src/cli/services/app-security-engine/tests/deterministic-rules.test.ts

@@ -29,6 +29,7 @@ const ACTIVE_IDS = [
   'UNSAFE_INNERHTML',
   'APP_PROXY_LIQUID_INJECTION',
   'STATIC_FRAME_ANCESTORS',
+  'THEME_EXTENSION_UNDECLARED_EGRESS',
 ].sort()
 
 const source = (content: string, path = 'app/routes/example.tsx'): SourceFile => ({
@@ -61,6 +62,7 @@ describe('deterministic rules product contract', () => {
     expect(DETERMINISTIC_CHECKS.get('LIQUID_UNSAFE_RENDER')?.version).toBe(2)
     expect(DETERMINISTIC_CHECKS.get('STATIC_FRAME_ANCESTORS')?.version).toBe(2)
     expect(DETERMINISTIC_CHECKS.get('MISSING_DEPENDENCY_SECURITY_AUTOMATION')?.version).toBe(2)
+    expect(DETERMINISTIC_CHECKS.get('THEME_EXTENSION_UNDECLARED_EGRESS')?.version).toBe(1)
   })
 
   test('extracts security fields from parsed TOML without source regexes', () => {

packages/app/src/cli/services/app-security-engine/tests/registry.test.ts

@@ -21,6 +21,7 @@ const PREFER_AGENT_CHECK_IDS = [
 const EXPLICIT_UNION_CHECK_IDS = [
   'COMMITTED_SECRET',
   'LIQUID_UNSAFE_RENDER',
+  'THEME_EXTENSION_UNDECLARED_EGRESS',
   'UNAUTHENTICATED_ENDPOINT',
   'UNSAFE_INNERHTML',
 ]
@@ -66,7 +67,7 @@ describe('check versions', () => {
 })
 
 describe('check precedence', () => {
-  test('sets the expected precedence on the 14 shared checks and defaults every other check to union', () => {
+  test('sets the expected precedence on the 15 shared checks and defaults every other check to union', () => {
     const checks = loadChecks()
     const precedenceById = new Map<string, CheckPrecedence>()
     for (const check of checks.values()) precedenceById.set(check.id, check.precedence)
@@ -75,7 +76,7 @@ describe('check precedence', () => {
       ...PREFER_AGENT_CHECK_IDS.map((id): [string, CheckPrecedence] => [id, 'prefer-agent']),
       ...EXPLICIT_UNION_CHECK_IDS.map((id): [string, CheckPrecedence] => [id, 'union']),
     ])
-    expect(expected.size).toBe(14)
+    expect(expected.size).toBe(15)
     for (const [id, precedence] of expected) expect(precedenceById.get(id), id).toBe(precedence)
 
     const others = [...checks.values()].filter((check) => !expected.has(check.id))