Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/itchy-jobs-report.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/theme': patch
---

Fix theme dev proxy to support SFAPI requests.
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
patchRenderingResponse,
proxyStorefrontRequest,
} from './proxy.js'
import {describe, test, expect} from 'vitest'
import {describe, test, expect, vi, beforeEach, afterEach} from 'vitest'
import {createEvent} from 'h3'
import {IncomingMessage, ServerResponse} from 'node:http'

Expand Down Expand Up @@ -404,4 +404,78 @@ describe('dev proxy', () => {
)
})
})

describe('proxyStorefrontRequest — Storefront API passthrough', () => {
const passthroughCtx = {
...ctx,
type: 'theme',
session: {
storeFqdn: 'my-store.myshopify.com',
sessionCookies: {_shopify_essential: 'essential-value'},
storefrontToken: 'sfr-devtools-token',
},
} as unknown as DevServerContext

let fetchMock: ReturnType<typeof vi.fn>

beforeEach(() => {
fetchMock = vi.fn().mockResolvedValue(new Response('{"data":{}}'))
vi.stubGlobal('fetch', fetchMock)
})

afterEach(() => {
vi.unstubAllGlobals()
})

test('forwards /api/YYYY-MM/graphql.json without injecting theme auth, cookies, referer, or dev params', async () => {
const event = createH3Event('POST', '/api/2026-01/graphql.json', {
'x-shopify-storefront-access-token': 'public-access-token',
authorization: 'Bearer client-supplied-token',
})

await proxyStorefrontRequest(event, passthroughCtx)

expect(fetchMock).toHaveBeenCalledOnce()
const [requestUrl, init] = fetchMock.mock.calls[0] as [URL, RequestInit]

expect(requestUrl.toString()).toBe('https://my-store.myshopify.com/api/2026-01/graphql.json')
expect(requestUrl.searchParams.has('_fd')).toBe(false)
expect(requestUrl.searchParams.has('pb')).toBe(false)

const headers = init.headers as Record<string, string>
expect(headers['x-shopify-storefront-access-token']).toBe('public-access-token')
expect(headers.authorization).toBe('Bearer client-supplied-token')
expect(headers.Authorization).toBeUndefined()
expect(headers.Cookie).toBeUndefined()
expect(headers.referer).toBeUndefined()
})
Comment on lines +430 to +451
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The passthrough tests currently assert headers.Cookie/headers.Authorization are undefined, but getProxyStorefrontHeaders() produces lowercased keys (see existing snapshot in this file), so these assertions don’t prove that cookies/authorization aren’t being forwarded (they could still be present as headers.cookie / headers.authorization). If the goal is to ensure nothing auth/cookie-like is forwarded for SFAPI, update the test to set cookie/referer/authorization on the incoming request and assert both casings are absent (or, if forwarding caller auth is intended, clarify expectations and ensure the proxy doesn’t overwrite it).

Copilot uses AI. Check for mistakes.

test('forwards /api/unstable/graphql.json through the passthrough path', async () => {
const event = createH3Event('POST', '/api/unstable/graphql.json')

await proxyStorefrontRequest(event, passthroughCtx)

expect(fetchMock).toHaveBeenCalledOnce()
const [requestUrl, init] = fetchMock.mock.calls[0] as [URL, RequestInit]

expect(requestUrl.toString()).toBe('https://my-store.myshopify.com/api/unstable/graphql.json')
const headers = init.headers as Record<string, string>
expect(headers.Authorization).toBeUndefined()
expect(headers.Cookie).toBeUndefined()
})

test('does not passthrough non-matching paths (e.g. /api/2026-01/graphql.js) — falls back to SFR auth injection', async () => {
const event = createH3Event('POST', '/api/2026-01/graphql.js')

await proxyStorefrontRequest(event, passthroughCtx)

expect(fetchMock).toHaveBeenCalledOnce()
const [requestUrl, init] = fetchMock.mock.calls[0] as [URL, RequestInit]

expect(requestUrl.searchParams.get('_fd')).toBe('0')
expect(requestUrl.searchParams.get('pb')).toBe('0')
const headers = init.headers as Record<string, string>
expect(headers.Authorization).toBe('Bearer sfr-devtools-token')
})
})
})
55 changes: 32 additions & 23 deletions packages/theme/src/cli/utilities/theme-environment/proxy.ts
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before this gets merged let's get @EvilGenius13 to look at it who has done a bunch of header stuff recently to prevent 401 and 429s.

Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const CHECKOUT_PATTERN = /^\/checkouts\/(?!internal\/)/
const ACCOUNT_PATTERN = /^\/account(\/login\/multipass(\/[^/]+)?|\/logout)?\/?$/
const VANITY_CDN_PATTERN = new RegExp(`^${VANITY_CDN_PREFIX}`)
const EXTENSION_CDN_PATTERN = new RegExp(`^${EXTENSION_CDN_PREFIX}`)
const STOREFRONT_API_PATTERN = /^\/api\/(unstable|\d{4}-\d{2})\/graphql\.json/

const IGNORED_ENDPOINTS = [
'/.well-known',
Expand Down Expand Up @@ -118,6 +119,16 @@ function getStoreFqdnForRegEx(ctx: DevServerContext) {
return ctx.session.storeFqdn.replace(/\\/g, '\\\\').replace(/\./g, '\\.')
}

/**
* Whether the request should be forwarded to SFR without modification.
*/
function isPassthroughRequest(event: H3Event) {
// Forward Storefront API requests as-is. The public Storefront API expects
// X-Shopify-Storefront-Access-Token from the caller and rejects our SFR
// devtools bearer, so we must not inject theme auth, cookies, or dev params.
return STOREFRONT_API_PATTERN.test(event.path)
}

/**
* Replaces every VanityCDN-like (...myshopify.com/cdn/...) URL to pass through the local server.
* It also replaces MainCDN-like (cdn.shopify.com/...) URLs to files that are known local assets.
Expand Down Expand Up @@ -306,41 +317,39 @@ export function proxyStorefrontRequest(event: H3Event, ctx: DevServerContext): P
)
}

// When a .css.liquid or .js.liquid file is requested but it doesn't exist in SFR,
// it will be rendered with a query string like `assets/file.css?1234`.
// For some reason, after refreshing, this rendered URL keeps the wrong `?1234`
// query string for a while. We replace it with a proper timestamp here to fix it.
if (/\/assets\/[^/]+\.(css|js)$/.test(url.pathname) && /\?\d+$/.test(url.search)) {
url.search = `?v=${Date.now()}`
}

url.searchParams.set('_fd', '0')
url.searchParams.set('pb', '0')
const headers = getProxyStorefrontHeaders(event)
const body = getRequestWebStream(event)
let headers = getProxyStorefrontHeaders(event)

if (!isPassthroughRequest(event)) {
// When a .css.liquid or .js.liquid file is requested but it doesn't exist in SFR,
// it will be rendered with a query string like `assets/file.css?1234`.
// For some reason, after refreshing, this rendered URL keeps the wrong `?1234`
// query string for a while. We replace it with a proper timestamp here to fix it.
if (/\/assets\/[^/]+\.(css|js)$/.test(url.pathname) && /\?\d+$/.test(url.search)) {
url.search = `?v=${Date.now()}`
}

const baseHeaders: Record<string, string> = {
...headers,
...defaultHeaders(),
referer: url.origin,
Cookie: buildCookies(ctx.session, {headers}),
}
url.searchParams.set('_fd', '0')
url.searchParams.set('pb', '0')

// Only include Authorization for theme dev, not theme-extensions
if (ctx.type === 'theme') {
baseHeaders.Authorization = `Bearer ${ctx.session.storefrontToken}`
headers = cleanHeader({
...headers,
...defaultHeaders(),
referer: url.origin,
Cookie: buildCookies(ctx.session, {headers}),
// Only include Authorization for theme dev, not theme-extensions
...(ctx.type === 'theme' ? {Authorization: `Bearer ${ctx.session.storefrontToken}`} : {}),
})
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isPassthroughRequest skips the injection block, but the passthrough path still forwards any client-supplied cookie, authorization, and referer headers (from getProxyStorefrontHeaders(event)), and also forwards _fd/pb if they were already present in the incoming URL. This contradicts the stated intent of not sending auth/cookies/dev params to the public Storefront API, and can also leak caller Authorization tokens upstream. Consider explicitly deleting cookie/Cookie, authorization/Authorization, and referer from headers (and deleting _fd/pb from url.searchParams) when isPassthroughRequest(event) is true.

Suggested change
})
})
} else {
delete headers.cookie
delete headers.Cookie
delete headers.authorization
delete headers.Authorization
delete headers.referer
delete headers.Referer
url.searchParams.delete('_fd')
url.searchParams.delete('pb')

Copilot uses AI. Check for mistakes.
}

const finalHeaders = cleanHeader(baseHeaders)

// eslint-disable-next-line no-restricted-globals
return fetch(url, {
method: event.method,
body,
duplex: body ? 'half' : undefined,
// Important to return 3xx responses to the client
redirect: 'manual',
headers: finalHeaders,
headers,
} as RequestInit & {duplex?: 'half'})
.then((response) => patchProxiedResponseHeaders(ctx, response))
.catch((error: Error) => {
Expand Down
Loading