From d0e135b8fde61e547d1b2cca11240d320b09beb9 Mon Sep 17 00:00:00 2001 From: Kara Daviduik Date: Tue, 10 Feb 2026 01:26:41 -0500 Subject: [PATCH 1/2] fix(theme-dev): cart AJAX endpoints no longer return 401 Unauthorized Cart AJAX endpoints (/cart/add.js, /cart/update.js, /cart/change.js, /cart.js, /cart.json) return 401 when running `shopify theme dev` on CLI 3.89.0, but work correctly on 3.88.0. 5+ developers affected. Root cause: Commit dda10ad2b2 added `Authorization: Bearer ` to all proxied requests. Cart endpoints authenticate via session cookies (_shopify_essential). When SFR's BearerTokenAuthorizationMiddleware sees the Authorization header, it selects token-based auth instead of cookie auth. The CLI's Storefront API token lacks cart scopes (write_global_api_universal_cart), so GlobalApi::Authorize.perform() fails with 401. Fix: Added needsBearerToken() helper that returns false for cart, checkout, and account paths (session-cookie auth), and true for CDN/asset paths (need Bearer token for theme preview rendering). Key decisions: - Denylist (exclude specific paths) over allowlist because the set of cookie-auth endpoints is small/stable while Bearer-needing paths are open-ended. An allowlist would risk silent rendering breakage. - Separate CART_SESSION_PATTERN from CART_PATTERN because routing and auth have different scopes (routing excludes GET /cart to render locally; auth excludes all cart paths from Bearer tokens). - Reuses CHECKOUT_PATTERN and ACCOUNT_PATTERN from routing since their routing and auth scopes currently align. --- .changeset/fix-cart-ajax-401.md | 5 + .../utilities/theme-environment/proxy.test.ts | 123 +++++++++++++++++- .../cli/utilities/theme-environment/proxy.ts | 20 ++- 3 files changed, 145 insertions(+), 3 deletions(-) create mode 100644 .changeset/fix-cart-ajax-401.md diff --git a/.changeset/fix-cart-ajax-401.md b/.changeset/fix-cart-ajax-401.md new file mode 100644 index 0000000000..4e1ba41513 --- /dev/null +++ b/.changeset/fix-cart-ajax-401.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': patch +--- + +Fix 401 Unauthorized errors on cart AJAX endpoints during `shopify theme dev` 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 dd954934a9..712a67f70b 100644 --- a/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts @@ -5,7 +5,7 @@ import { patchRenderingResponse, proxyStorefrontRequest, } from './proxy.js' -import {describe, test, expect} from 'vitest' +import {describe, test, expect, vi, afterEach} from 'vitest' import {createEvent} from 'h3' import {IncomingMessage, ServerResponse} from 'node:http' import {Socket} from 'node:net' @@ -401,5 +401,126 @@ describe('dev proxy', () => { 'Request failed: Hostname mismatch. Expected host: cdn.shopify.com. Resulting URL hostname: evil.com', ) }) + + describe('Authorization header behavior', () => { + const themeCtx = { + session: { + storeFqdn: 'my-store.myshopify.com', + storefrontToken: 'test-sfr-token', + storefrontPassword: '', + sessionCookies: {}, + }, + options: {host: 'localhost', port: '9292'}, + localThemeFileSystem: {files: new Map()}, + localThemeExtensionFileSystem: {files: new Map()}, + type: 'theme', + } as unknown as DevServerContext + + const extensionCtx = { + ...themeCtx, + type: 'theme-extension', + } as unknown as DevServerContext + + let fetchMock: ReturnType + + afterEach(() => { + vi.unstubAllGlobals() + }) + + function stubFetchAndProxy(method: string, path: string, context: DevServerContext) { + fetchMock = vi.fn().mockResolvedValue(new Response('ok')) + vi.stubGlobal('fetch', fetchMock) + const event = createH3Event(method, path) + return proxyStorefrontRequest(event, context) + } + + function getAuthHeader(): string | undefined { + const headers = fetchMock.mock.calls[0]![1].headers as Record + return headers.Authorization ?? headers.authorization + } + + describe('excludes Bearer token for session-cookie-auth paths', () => { + test('POST /cart/add.js', async () => { + await stubFetchAndProxy('POST', '/cart/add.js', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) + + test('POST /cart/update.js', async () => { + await stubFetchAndProxy('POST', '/cart/update.js', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) + + test('GET /cart.js', async () => { + await stubFetchAndProxy('GET', '/cart.js', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) + + test('GET /cart.json', async () => { + await stubFetchAndProxy('GET', '/cart.json', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) + + test('POST /cart (bare)', async () => { + await stubFetchAndProxy('POST', '/cart', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) + + test('POST /cart/change.js', async () => { + await stubFetchAndProxy('POST', '/cart/change.js', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) + + test('GET /checkouts/abc123', async () => { + await stubFetchAndProxy('GET', '/checkouts/abc123', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) + + test('GET /account/logout', async () => { + await stubFetchAndProxy('GET', '/account/logout', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) + }) + + describe('excludes Bearer token when path has query strings', () => { + test('GET /cart.js?sections=header', async () => { + await stubFetchAndProxy('GET', '/cart.js?sections=header', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) + + test('POST /cart/add.js?sections=main-cart-items', async () => { + await stubFetchAndProxy('POST', '/cart/add.js?sections=main-cart-items', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) + + test('GET /account?foo=bar', async () => { + await stubFetchAndProxy('GET', '/account?foo=bar', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) + }) + + describe('includes Bearer token for CDN and asset paths', () => { + test('GET /cdn/shop/t/10/assets/theme.css', async () => { + await stubFetchAndProxy('GET', '/cdn/shop/t/10/assets/theme.css', themeCtx) + expect(getAuthHeader()).toBe('Bearer test-sfr-token') + }) + + test('GET /some-path.js', async () => { + await stubFetchAndProxy('GET', '/some-path.js', themeCtx) + expect(getAuthHeader()).toBe('Bearer test-sfr-token') + }) + + test('GET /checkouts/internal/something (negative lookahead boundary)', async () => { + await stubFetchAndProxy('GET', '/checkouts/internal/something', themeCtx) + expect(getAuthHeader()).toBe('Bearer test-sfr-token') + }) + }) + + describe('excludes Bearer token for theme-extension context', () => { + test('GET /cdn/shop/t/10/assets/theme.css with theme-extension type', async () => { + await stubFetchAndProxy('GET', '/cdn/shop/t/10/assets/theme.css', extensionCtx) + expect(getAuthHeader()).toBeUndefined() + }) + }) + }) }) }) diff --git a/packages/theme/src/cli/utilities/theme-environment/proxy.ts b/packages/theme/src/cli/utilities/theme-environment/proxy.ts index 9f08555eb5..a2d2e81b6b 100644 --- a/packages/theme/src/cli/utilities/theme-environment/proxy.ts +++ b/packages/theme/src/cli/utilities/theme-environment/proxy.ts @@ -16,6 +16,9 @@ export const EXTENSION_CDN_PREFIX = '/ext/cdn/' const API_COLLECT_PATH = '/api/collect' const CART_PATTERN = /^\/cart\// +// Broader than CART_PATTERN (routing) -- covers /cart, /cart.js, /cart.json for auth exclusion. +// CART_PATTERN only matches /cart/... subpaths to avoid proxying GET /cart (the cart HTML page). +const CART_SESSION_PATTERN = /^\/cart(\/|\.js|\.json|$)/ const CHECKOUT_PATTERN = /^\/checkouts\/(?!internal\/)/ const ACCOUNT_PATTERN = /^\/account(\/login\/multipass(\/[^/]+)?|\/logout)?\/?$/ const VANITY_CDN_PATTERN = new RegExp(`^${VANITY_CDN_PREFIX}`) @@ -114,6 +117,18 @@ export function canProxyRequest(event: H3Event) { return Boolean(extension) || acceptsType !== '*/*' } +// Cart, checkout, and account endpoints use session-cookie auth, not Bearer tokens. +// CHECKOUT_PATTERN and ACCOUNT_PATTERN are reused from routing because their +// routing and auth scopes currently align; changes to those patterns should +// consider the auth implications here. +function needsBearerToken(path: string): boolean { + const [pathname] = path.split('?') as [string] + if (CART_SESSION_PATTERN.test(pathname)) return false + if (CHECKOUT_PATTERN.test(pathname)) return false + if (ACCOUNT_PATTERN.test(pathname)) return false + return true +} + function getStoreFqdnForRegEx(ctx: DevServerContext) { return ctx.session.storeFqdn.replace(/\\/g, '\\\\').replace(/\./g, '\\.') } @@ -326,8 +341,9 @@ export function proxyStorefrontRequest(event: H3Event, ctx: DevServerContext): P Cookie: buildCookies(ctx.session, {headers}), } - // Only include Authorization for theme dev, not theme-extensions - if (ctx.type === 'theme') { + // Cart, checkout, and account endpoints use cookie-based auth. + // Sending a Bearer token causes SFR to select token auth, which lacks cart scopes. + if (ctx.type === 'theme' && needsBearerToken(event.path)) { baseHeaders.Authorization = `Bearer ${ctx.session.storefrontToken}` } From ac00b75ccf5a0954c7d085edf3c701f3152815d7 Mon Sep 17 00:00:00 2001 From: Kara Daviduik Date: Tue, 10 Feb 2026 02:18:37 -0500 Subject: [PATCH 2/2] fix(theme-dev): lint violations in proxy regression tests Flatten describe nesting to comply with vitest/max-nested-describe (max: 2) and use index signature style per consistent-indexed-object-style rule. --- .../utilities/theme-environment/proxy.test.ts | 214 +++++++++--------- 1 file changed, 102 insertions(+), 112 deletions(-) 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 712a67f70b..3e6bb409c5 100644 --- a/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts @@ -402,125 +402,115 @@ describe('dev proxy', () => { ) }) - describe('Authorization header behavior', () => { - const themeCtx = { - session: { - storeFqdn: 'my-store.myshopify.com', - storefrontToken: 'test-sfr-token', - storefrontPassword: '', - sessionCookies: {}, - }, - options: {host: 'localhost', port: '9292'}, - localThemeFileSystem: {files: new Map()}, - localThemeExtensionFileSystem: {files: new Map()}, - type: 'theme', - } as unknown as DevServerContext + const themeCtx = { + session: { + storeFqdn: 'my-store.myshopify.com', + storefrontToken: 'test-sfr-token', + storefrontPassword: '', + sessionCookies: {}, + }, + options: {host: 'localhost', port: '9292'}, + localThemeFileSystem: {files: new Map()}, + localThemeExtensionFileSystem: {files: new Map()}, + type: 'theme', + } as unknown as DevServerContext + + const extensionCtx = { + ...themeCtx, + type: 'theme-extension', + } as unknown as DevServerContext + + let fetchMock: ReturnType + + afterEach(() => { + vi.unstubAllGlobals() + }) - const extensionCtx = { - ...themeCtx, - type: 'theme-extension', - } as unknown as DevServerContext + function stubFetchAndProxy(method: string, path: string, context: DevServerContext) { + fetchMock = vi.fn().mockResolvedValue(new Response('ok')) + vi.stubGlobal('fetch', fetchMock) + const event = createH3Event(method, path) + return proxyStorefrontRequest(event, context) + } + + function getAuthHeader(): string | undefined { + const headers = fetchMock.mock.calls[0]![1].headers as {[key: string]: string} + return headers.Authorization ?? headers.authorization + } + + test('excludes Bearer token for POST /cart/add.js', async () => { + await stubFetchAndProxy('POST', '/cart/add.js', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) - let fetchMock: ReturnType + test('excludes Bearer token for POST /cart/update.js', async () => { + await stubFetchAndProxy('POST', '/cart/update.js', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) - afterEach(() => { - vi.unstubAllGlobals() - }) + test('excludes Bearer token for GET /cart.js', async () => { + await stubFetchAndProxy('GET', '/cart.js', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) - function stubFetchAndProxy(method: string, path: string, context: DevServerContext) { - fetchMock = vi.fn().mockResolvedValue(new Response('ok')) - vi.stubGlobal('fetch', fetchMock) - const event = createH3Event(method, path) - return proxyStorefrontRequest(event, context) - } - - function getAuthHeader(): string | undefined { - const headers = fetchMock.mock.calls[0]![1].headers as Record - return headers.Authorization ?? headers.authorization - } - - describe('excludes Bearer token for session-cookie-auth paths', () => { - test('POST /cart/add.js', async () => { - await stubFetchAndProxy('POST', '/cart/add.js', themeCtx) - expect(getAuthHeader()).toBeUndefined() - }) - - test('POST /cart/update.js', async () => { - await stubFetchAndProxy('POST', '/cart/update.js', themeCtx) - expect(getAuthHeader()).toBeUndefined() - }) - - test('GET /cart.js', async () => { - await stubFetchAndProxy('GET', '/cart.js', themeCtx) - expect(getAuthHeader()).toBeUndefined() - }) - - test('GET /cart.json', async () => { - await stubFetchAndProxy('GET', '/cart.json', themeCtx) - expect(getAuthHeader()).toBeUndefined() - }) - - test('POST /cart (bare)', async () => { - await stubFetchAndProxy('POST', '/cart', themeCtx) - expect(getAuthHeader()).toBeUndefined() - }) - - test('POST /cart/change.js', async () => { - await stubFetchAndProxy('POST', '/cart/change.js', themeCtx) - expect(getAuthHeader()).toBeUndefined() - }) - - test('GET /checkouts/abc123', async () => { - await stubFetchAndProxy('GET', '/checkouts/abc123', themeCtx) - expect(getAuthHeader()).toBeUndefined() - }) - - test('GET /account/logout', async () => { - await stubFetchAndProxy('GET', '/account/logout', themeCtx) - expect(getAuthHeader()).toBeUndefined() - }) - }) + test('excludes Bearer token for GET /cart.json', async () => { + await stubFetchAndProxy('GET', '/cart.json', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) - describe('excludes Bearer token when path has query strings', () => { - test('GET /cart.js?sections=header', async () => { - await stubFetchAndProxy('GET', '/cart.js?sections=header', themeCtx) - expect(getAuthHeader()).toBeUndefined() - }) - - test('POST /cart/add.js?sections=main-cart-items', async () => { - await stubFetchAndProxy('POST', '/cart/add.js?sections=main-cart-items', themeCtx) - expect(getAuthHeader()).toBeUndefined() - }) - - test('GET /account?foo=bar', async () => { - await stubFetchAndProxy('GET', '/account?foo=bar', themeCtx) - expect(getAuthHeader()).toBeUndefined() - }) - }) + test('excludes Bearer token for POST /cart (bare)', async () => { + await stubFetchAndProxy('POST', '/cart', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) - describe('includes Bearer token for CDN and asset paths', () => { - test('GET /cdn/shop/t/10/assets/theme.css', async () => { - await stubFetchAndProxy('GET', '/cdn/shop/t/10/assets/theme.css', themeCtx) - expect(getAuthHeader()).toBe('Bearer test-sfr-token') - }) - - test('GET /some-path.js', async () => { - await stubFetchAndProxy('GET', '/some-path.js', themeCtx) - expect(getAuthHeader()).toBe('Bearer test-sfr-token') - }) - - test('GET /checkouts/internal/something (negative lookahead boundary)', async () => { - await stubFetchAndProxy('GET', '/checkouts/internal/something', themeCtx) - expect(getAuthHeader()).toBe('Bearer test-sfr-token') - }) - }) + test('excludes Bearer token for POST /cart/change.js', async () => { + await stubFetchAndProxy('POST', '/cart/change.js', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) - describe('excludes Bearer token for theme-extension context', () => { - test('GET /cdn/shop/t/10/assets/theme.css with theme-extension type', async () => { - await stubFetchAndProxy('GET', '/cdn/shop/t/10/assets/theme.css', extensionCtx) - expect(getAuthHeader()).toBeUndefined() - }) - }) + test('excludes Bearer token for GET /checkouts/abc123', async () => { + await stubFetchAndProxy('GET', '/checkouts/abc123', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) + + test('excludes Bearer token for GET /account/logout', async () => { + await stubFetchAndProxy('GET', '/account/logout', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) + + test('excludes Bearer token for GET /cart.js?sections=header (query string)', async () => { + await stubFetchAndProxy('GET', '/cart.js?sections=header', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) + + test('excludes Bearer token for POST /cart/add.js?sections=main-cart-items (query string)', async () => { + await stubFetchAndProxy('POST', '/cart/add.js?sections=main-cart-items', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) + + test('excludes Bearer token for GET /account?foo=bar (query string with $ anchor)', async () => { + await stubFetchAndProxy('GET', '/account?foo=bar', themeCtx) + expect(getAuthHeader()).toBeUndefined() + }) + + test('includes Bearer token for GET /cdn/shop/t/10/assets/theme.css', async () => { + await stubFetchAndProxy('GET', '/cdn/shop/t/10/assets/theme.css', themeCtx) + expect(getAuthHeader()).toBe('Bearer test-sfr-token') + }) + + test('includes Bearer token for GET /some-path.js', async () => { + await stubFetchAndProxy('GET', '/some-path.js', themeCtx) + expect(getAuthHeader()).toBe('Bearer test-sfr-token') + }) + + test('includes Bearer token for GET /checkouts/internal/something (negative lookahead)', async () => { + await stubFetchAndProxy('GET', '/checkouts/internal/something', themeCtx) + expect(getAuthHeader()).toBe('Bearer test-sfr-token') + }) + + test('excludes Bearer token for theme-extension context', async () => { + await stubFetchAndProxy('GET', '/cdn/shop/t/10/assets/theme.css', extensionCtx) + expect(getAuthHeader()).toBeUndefined() }) }) })