Skip to content

Conversation

@EvilGenius13
Copy link
Contributor

@EvilGenius13 EvilGenius13 commented Nov 11, 2025

WHY are these changes introduced?

theme dev had a lax CORS policy that could allow any website to read authenticated data from a developer's local server.

WHAT is this pull request doing?

Two part fix:

Part 1:
Restrict Cors in the dev server middleware.

  • Created an allowed origins list that consists of the local server address localhost:port and the production store example-shop.myshopify.com
  • Allowed methods are GET, PUT, HEAD, OPTIONS.
  • Added credentials: true as this forces the server to be explicit about which origin(s) is allowed.
  • We only apply this change when an origin header is present. This is because the browser only sends that header when it's making a cross-origin request from another domain.

Part 2:
Strip the CORS headers from proxied responses

  • During testing I saw that we were getting responses back with Access-Control-Allow-Origin: *.
  • We delete any headers involving CORS and our CORS middleware sets the correct headers.

How to test your changes?

Build the branch and run theme dev

Open up your browser to a website that isn't your store or localhost. (i.e google.com) and open up the inspector. Open the console and paste

fetch('http://127.0.0.1:9292/products.json')
  .then(r => r.json())
  .then(data => console.log('✅ SUCCESS:', data))
  .catch(err => console.error('❌ BLOCKED:', err.message))

You should see blocked. You can try this with a current version of the CLI and it should pass (which we don't want).

Post-release steps

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

@EvilGenius13 EvilGenius13 requested review from a team as code owners November 11, 2025 21:36
@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 11, 2025

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
79.2% (-0.03% 🔻)
13609/17184
🟡 Branches
73.1% (-0.01% 🔻)
6634/9075
🟡 Functions
79.32% (-0.05% 🔻)
3509/4424
🟡 Lines
79.56% (-0.02% 🔻)
12854/16157
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / bulk-operation-run-query.ts
100% 100% 100% 100%
🟢
... / execute-bulk-operation.ts
100% 83.33% 100% 100%
🟢
... / run-query.ts
100% 100% 100% 100%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / ConcurrentOutput.tsx
98.36% (-1.64% 🔻)
92% (-4% 🔻)
100%
98.33% (-1.67% 🔻)
🔴
... / ui.tsx
50.82% (-0.79% 🔻)
42.86% (-5.53% 🔻)
54.55% (+1.42% 🔼)
50% (-0.82% 🔻)
🟡
... / theme-environment.ts
69.57% (-1.86% 🔻)
50%
55.56% (-3.27% 🔻)
69.57% (-1.86% 🔻)

Test suite run success

3360 tests passing in 1376 suites.

Report generated by 🧪jest coverage report action from 226b49e

@shopify-security-bot
Copy link

Hi team!

This security issue has been categorized as a SEV-2 according to the Shopify Issue Severity Standard. This type of issue has an associated resolution timeline of 2 weeks, making the resolution target for this issue November 25, 2025 21:36 UTC (12 days from now).

If this is a package bump which is passing CI, then Dependabot Auto-Merge will likely attempt to merge it on your behalf. Please review the Dependabot Auto-Merge FAQ to learn more.

If you have any questions or believe that this resolution timeline will not be possible, please reach out to us in #help-appsec.

@EvilGenius13
Copy link
Contributor Author

/snapit

@github-actions
Copy link
Contributor

🫰✨ Thanks @EvilGenius13! Your snapshot has been published to npm.

Test the snapshot by installing your package globally:

npm i -g --@shopify:registry=https://registry.npmjs.org @shopify/[email protected]

Caution

After installing, validate the version by running just shopify in your terminal.
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

Copy link
Contributor

@isaacroldan isaacroldan left a comment

Choose a reason for hiding this comment

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

Tested locally and works as expected 👌

Copy link
Contributor

@graygilmore graygilmore left a comment

Choose a reason for hiding this comment

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

🎩 worked!

@shopify-security-bot
Copy link

Hi team!

This security issue has been categorized as a SEV-3 according to the Shopify Issue Severity Standard. This type of issue has an associated resolution timeline of 3 months, making the resolution target for this issue February 11, 2026 21:36 UTC (3 months from now).

If this is a package bump which is passing CI, then Dependabot Auto-Merge will likely attempt to merge it on your behalf. Please review the Dependabot Auto-Merge FAQ to learn more.

If you have any questions or believe that this resolution timeline will not be possible, please reach out to us in #help-appsec.

@EvilGenius13 EvilGenius13 force-pushed the cors-theme-dev-update branch 2 times, most recently from ef68e02 to 6a5f776 Compare November 18, 2025 19:41
Copy link
Contributor

@frandiox frandiox left a comment

Choose a reason for hiding this comment

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

Thanks!

@EvilGenius13 EvilGenius13 added this pull request to the merge queue Nov 19, 2025
Merged via the queue into main with commit bf902a4 Nov 19, 2025
25 checks passed
@EvilGenius13 EvilGenius13 deleted the cors-theme-dev-update branch November 19, 2025 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants