Merge branch 'main' into fd-fix-inject-proxy

.changeset/hip-chefs-dance.md

@@ -0,0 +1,5 @@
+---
+'@shopify/theme': patch
+---
+
+Fix Theme Access authentication on `shopify theme dev` and `shopify theme console` commands

.changeset/sharp-eggs-remember.md

@@ -0,0 +1,5 @@
+---
+'@shopify/theme': patch
+---
+
+Fix cart/add request in development.

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

@@ -1,4 +1,5 @@
 import {loadLocalExtensionsSpecifications} from './load-specifications.js'
+import {createContractBasedModuleSpecification} from './specification.js'
 import {AppSchema} from '../app/app.js'
 import {describe, test, expect, beforeAll} from 'vitest'
 
@@ -26,3 +27,20 @@ describe('allLocalSpecs', () => {
     expect(got.length).not.toEqual(0)
   })
 })
+
+describe('createContractBasedModuleSpecification', () => {
+  test('creates a specification with the given identifier', () => {
+    // When
+    const got = createContractBasedModuleSpecification('test', ['bundling'])
+
+    // Then
+    expect(got).toMatchObject(
+      expect.objectContaining({
+        identifier: 'test',
+        experience: 'extension',
+        uidStrategy: 'uuid',
+      }),
+    )
+    expect(got.appModuleFeatures()).toEqual(['bundling'])
+  })
+})

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

@@ -263,6 +263,20 @@ export function createContractBasedConfigModuleSpecification<TKey extends string
   })
 }
 
+export function createContractBasedModuleSpecification<TConfiguration extends BaseConfigType = BaseConfigType>(
+  identifier: string,
+  appModuleFeatures?: ExtensionFeature[],
+) {
+  return createExtensionSpecification({
+    identifier,
+    schema: zod.any({}) as unknown as ZodSchemaType<TConfiguration>,
+    appModuleFeatures: () => appModuleFeatures ?? [],
+    deployConfig: async (config, _) => {
+      return config
+    },
+  })
+}
+
 function resolveAppConfigTransform(transformConfig?: TransformationConfig | CustomTransformationConfig) {
   if (!transformConfig) return (content: object) => defaultAppConfigTransform(content as {[key: string]: unknown})
 

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

@@ -1,30 +1,5 @@
-import {MarketingActivityExtensionSchema} from './marketing_activity_schemas/marketing_activity_schema.js'
-import {createExtensionSpecification} from '../specification.js'
-import {randomUUID} from '@shopify/cli-kit/node/crypto'
+import {createContractBasedModuleSpecification} from '../specification.js'
 
-const spec = createExtensionSpecification({
-  identifier: 'marketing_activity',
-  schema: MarketingActivityExtensionSchema,
-  appModuleFeatures: (_) => ['bundling'],
-  deployConfig: async (config, _) => {
-    return {
-      title: config.title,
-      description: config.description,
-      api_path: config.api_path,
-      tactic: config.tactic,
-      marketing_channel: config.marketing_channel,
-      referring_domain: config.referring_domain,
-      is_automation: config.is_automation,
-      use_external_editor: config.use_external_editor,
-      preview_data: config.preview_data,
-      fields: config.fields.map((field) => ({
-        ...field,
-        // NOTE: we're not using this id anywhere, generating it to satisfy the schema
-        // decided not to remove it from the schema for now to minimize the risk of breaking changes
-        id: randomUUID(),
-      })),
-    }
-  },
-})
+const spec = createContractBasedModuleSpecification('marketing_activity')
 
 export default spec

packages/theme/src/cli/utilities/theme-environment/dev-server-session.ts

@@ -50,7 +50,11 @@ async function fetchDevServerSession(
 
   const session = await ensureAuthenticatedThemes(adminSession.storeFqdn, adminPassword, [])
   const storefrontToken = await ensureAuthenticatedStorefront([], adminPassword)
-  const sessionCookies = await getStorefrontSessionCookies(baseUrl, themeId, storefrontPassword, {})
+  const sessionCookies = await getStorefrontSessionCookies(baseUrl, themeId, storefrontPassword, {
+    'X-Shopify-Shop': session.storeFqdn,
+    'X-Shopify-Access-Token': session.token,
+    Authorization: `Bearer ${storefrontToken}`,
+  })
 
   return {
     ...session,

packages/theme/src/cli/utilities/theme-environment/html.ts

@@ -3,8 +3,10 @@ import {getInMemoryTemplates, injectHotReloadScript} from './hot-reload/server.j
 import {render} from './storefront-renderer.js'
 import {getExtensionInMemoryTemplates} from '../theme-ext-environment/theme-ext-server.js'
 import {defineEventHandler, getCookie, setResponseHeader, setResponseStatus, type H3Error} from 'h3'
-import {renderError} from '@shopify/cli-kit/node/ui'
+import {renderError, renderFatalError} from '@shopify/cli-kit/node/ui'
 import {outputInfo} from '@shopify/cli-kit/node/output'
+import {AbortError} from '@shopify/cli-kit/node/error'
+import type {Response} from '@shopify/cli-kit/node/http'
 import type {Theme} from '@shopify/cli-kit/node/themes/types'
 import type {DevServerContext} from './types.js'
 
@@ -29,6 +31,8 @@ export function getHtmlHandler(theme: Theme, ctx: DevServerContext) {
 
         html = prettifySyntaxErrors(html)
 
+        assertThemeId(response, html, String(theme.id))
+
         if (ctx.options.liveReload !== 'off') {
           html = injectHotReloadScript(html)
         }
@@ -50,7 +54,7 @@ export function getHtmlHandler(theme: Theme, ctx: DevServerContext) {
         let errorPageHtml = getErrorPage({
           title,
           header: title,
-          message: [...rest, error.message].join('<br>'),
+          message: [...rest, cause?.message ?? error.message].join('<br>'),
           code: error.stack?.replace(`${error.message}\n`, '') ?? '',
         })
 
@@ -98,3 +102,33 @@ function getErrorPage(options: {title: string; header: string; message: string;
     </body>
   </html>`
 }
+
+function assertThemeId(response: Response, html: string, expectedThemeId: string) {
+  /**
+   * DOM example:
+   *
+   * ```
+   * <script>var Shopify = Shopify || {};
+   * Shopify.locale = "en";
+   * Shopify.theme = {"name":"Development","id":143509762348,"theme_store_id":null,"role":"development"};
+   * Shopify.theme.handle = "null";
+   * ...;</script>
+   * ```
+   */
+  const obtainedThemeId = html.match(/Shopify\.theme\s*=\s*{[^}]+?"id":\s*"?(\d+)"?(}|,)/)?.[1]
+
+  if (obtainedThemeId && obtainedThemeId !== expectedThemeId) {
+    renderFatalError(
+      new AbortError(
+        `Theme ID mismatch: expected ${expectedThemeId} but got ${obtainedThemeId}.` +
+          `\nRequest ID: ${response.headers.get('x-request-id')}` +
+          `\nURL: ${response.url}`,
+        `This is likely related to an issue in upstream Shopify APIs.` +
+          `\nPlease try again in a few minutes and report this issue:` +
+          `\nhttps://github.com/Shopify/cli/issues/new?template=bug-report.yml`,
+      ),
+    )
+
+    process.exit(1)
+  }
+}

packages/theme/src/cli/utilities/theme-environment/proxy.test.ts

@@ -200,16 +200,17 @@ describe('dev proxy', () => {
       // Removed:
       event.node.req.headers.connection = '...'
       event.node.req.headers['proxy-authenticate'] = '...'
-      event.node.req.headers.accept = 'text/html'
       event.node.req.headers.host = 'abnb'
       // Kept:
+      event.node.req.headers.accept = 'text/html'
       event.node.req.headers.cookie = 'oreo'
       event.node.req.headers['user-agent'] = 'vitest'
       event.node.req.headers['x-custom'] = 'true'
 
       expect(getProxyStorefrontHeaders(event)).toMatchInlineSnapshot(`
         {
           "X-Forwarded-For": "42",
+          "accept": "text/html",
           "cookie": "oreo",
           "user-agent": "vitest",
           "x-custom": "true",

packages/theme/src/cli/utilities/theme-environment/proxy.ts

@@ -4,7 +4,7 @@ import {
   defineEventHandler,
   clearResponseHeaders,
   sendProxy,
-  getProxyRequestHeaders,
+  getRequestHeaders,
   getRequestWebStream,
   getRequestIP,
   type H3Event,
@@ -167,7 +167,9 @@ const HOP_BY_HOP_HEADERS = [
   'trailer',
   'transfer-encoding',
   'upgrade',
+  'expect',
   'content-security-policy',
+  'host',
 ]
 
 function patchProxiedResponseHeaders(ctx: DevServerContext, event: H3Event, response: Response | NodeResponse) {
@@ -203,10 +205,8 @@ function patchProxiedResponseHeaders(ctx: DevServerContext, event: H3Event, resp
  * Filters headers to forward to SFR.
  */
 export function getProxyStorefrontHeaders(event: H3Event) {
-  const proxyRequestHeaders = getProxyRequestHeaders(event) as {[key: string]: string}
+  const proxyRequestHeaders = getRequestHeaders(event) as {[key: string]: string}
 
-  // H3 already removes most hop-by-hop request headers:
-  // https://github.com/unjs/h3/blob/ac6d83de2abe5411d4eaea8ecf2165ace16a65f3/src/utils/proxy.ts#L25
   for (const headerKey of HOP_BY_HOP_HEADERS) {
     delete proxyRequestHeaders[headerKey]
   }

packages/theme/src/cli/utilities/theme-environment/storefront-renderer.test.ts

@@ -12,7 +12,7 @@ vi.mock('@shopify/cli-kit/node/http', async () => {
   }
 })
 
-const successResponse = {ok: true, status: 200, headers: {get: vi.fn()}} as any
+const successResponse = {ok: true, status: 200, headers: {get: vi.fn(), delete: vi.fn()}} as any
 
 const session: DevServerSession = {
   token: 'admin_token_abc123',
@@ -50,6 +50,7 @@ describe('render', () => {
 
     // Then
     expect(response.status).toEqual(200)
+    expect(response.headers.delete).toBeCalled()
     expect(fetch).toHaveBeenCalledWith(
       'https://store.myshopify.com/products/1?_fd=0&pb=0',
       expect.objectContaining({
@@ -74,6 +75,7 @@ describe('render', () => {
 
     // Then
     expect(response.status).toEqual(200)
+    expect(response.headers.delete).toBeCalled()
     expect(fetch).toHaveBeenCalledWith(
       'https://theme-kit-access.shopifyapps.com/cli/sfr/products/1?_fd=0&pb=0',
       expect.objectContaining({
@@ -109,6 +111,7 @@ describe('render', () => {
 
     // Then
     expect(response.status).toEqual(200)
+    expect(response.headers.delete).toBeCalled()
     expect(fetch).toHaveBeenCalledWith(
       'https://store.myshopify.com/products/1?_fd=0&pb=0&section_id=sections--1__announcement-bar',
       expect.objectContaining({
@@ -135,6 +138,7 @@ describe('render', () => {
 
     // Then
     expect(response.status).toEqual(200)
+    expect(response.headers.delete).toBeCalled()
     expect(fetch).toHaveBeenCalledWith(
       'https://store.myshopify.com/products/1?_fd=0&pb=0&app_block_id=00001111222233334444',
       expect.objectContaining({
@@ -162,6 +166,7 @@ describe('render', () => {
 
     // Then
     expect(response.status).toEqual(200)
+    expect(response.headers.delete).toBeCalled()
     expect(fetch).toHaveBeenCalledWith(
       'https://store.myshopify.com/products/1?_fd=0&pb=0&section_id=sections--1__announcement-bar',
       expect.objectContaining({
@@ -191,6 +196,7 @@ describe('render', () => {
 
     // Then
     expect(response.status).toEqual(200)
+    expect(response.headers.delete).toBeCalled()
     expect(fetch).toHaveBeenCalledWith(
       'https://store.myshopify.com/products/1?_fd=0&pb=0&value=A&value=B',
       expect.objectContaining({

packages/theme/src/cli/utilities/theme-environment/storefront-renderer.ts

@@ -34,6 +34,13 @@ export async function render(session: DevServerSession, context: DevServerRender
   const requestId = response.headers.get('x-request-id')
   outputDebug(`← ${response.status} (request_id: ${requestId})`)
 
+  /**
+   * Theme Access app requests return the 'application/json' content type.
+   * However, patched renderings will never patch JSON requests; so we're
+   * consistently discarding the content type.
+   */
+  response.headers.delete('Content-Type')
+
   return response
 }