Skip to content

Commit bf902a4

Browse files
authored
Merge pull request #6607 from Shopify/cors-theme-dev-update
prevent possible CORS vulnerability
2 parents 22f2bc6 + 226b49e commit bf902a4

File tree

3 files changed

+27
-5
lines changed

3 files changed

+27
-5
lines changed

.changeset/frank-lands-hunt.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/theme': patch
3+
---
4+
5+
Fix a possible CORS vulnerability where a website could read data from the localhost server

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,15 @@ function patchProxiedResponseHeaders(ctx: DevServerContext, rawResponse: Respons
228228
response.headers.delete(header)
229229
}
230230

231+
// Remove CORS headers from the proxied response. Our CORS middleware
232+
// will set the correct headers based on the allowed origins.
233+
response.headers.delete('access-control-allow-origin')
234+
response.headers.delete('access-control-allow-credentials')
235+
response.headers.delete('access-control-allow-methods')
236+
response.headers.delete('access-control-allow-headers')
237+
response.headers.delete('access-control-expose-headers')
238+
response.headers.delete('access-control-max-age')
239+
231240
// Link header preloads resources from global CDN, proxy it:
232241
const linkHeader = response.headers.get('Link')
233242
if (linkHeader) response.headers.set('Link', injectCdnProxy(linkHeader, ctx))

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,16 +98,24 @@ interface DevelopmentServerInstance {
9898

9999
function createDevelopmentServer(theme: Theme, ctx: DevServerContext, initialWork: Promise<void>) {
100100
const app = createApp()
101+
const allowedOrigins = [`http://${ctx.options.host}:${ctx.options.port}`, `https://${ctx.session.storeFqdn}`]
101102

102103
app.use(
103104
defineLazyEventHandler(async () => {
104105
await initialWork
105106
return defineEventHandler((event) => {
106-
handleCors(event, {
107-
origin: '*',
108-
methods: '*',
109-
preflight: {statusCode: 204},
110-
})
107+
const origin = event.node.req.headers.origin
108+
109+
// We only set CORS headers if an Origin header is present in the request.
110+
// This prevents wildcard CORS on direct navigation.
111+
if (origin) {
112+
handleCors(event, {
113+
origin: (requestOrigin) => allowedOrigins.includes(requestOrigin),
114+
credentials: true,
115+
methods: ['GET', 'POST', 'HEAD', 'OPTIONS'],
116+
preflight: {statusCode: 204},
117+
})
118+
}
111119
})
112120
}),
113121
)

0 commit comments

Comments
 (0)