Repository navigation
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation is bounded, preserves fallback behavior, and has comprehensive tests for success and failure paths.
Review effort: Balanced
Findings: None
What changed in this PR
Resolves permanent store domains for Theme Access authentication and improves 401 guidance.
Changes:
- Resolves and caches permanent domains via
/meta.json. - Uses resolved domains for Theme Access sessions.
- Adds targeted errors, tests, and release notes.
| File | Description |
|---|---|
packages/cli-kit/src/public/node/session.ts |
Resolves Theme Access session domains. |
packages/cli-kit/src/public/node/session.test.ts |
Tests session-domain resolution. |
packages/cli-kit/src/public/node/api/admin.ts |
Adds Theme Access 401 guidance. |
packages/cli-kit/src/public/node/api/admin.test.ts |
Tests the new error classification. |
packages/cli-kit/src/private/node/session/permanent-store-domain.ts |
Implements validated, cached domain lookup. |
packages/cli-kit/src/private/node/session/permanent-store-domain.test.ts |
Covers lookup, fallback, caching, and local behavior. |
.changeset/theme-access-permanent-domain.md |
Documents patch-level user-facing changes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
/snapit |
|
🫰✨ Thanks @graygilmore! Your snapshot has been published to npm. Built from Test the snapshot by installing your package globally: pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20261007155253Caution After installing, validate the version by running |
|
/snapit |
|
🫰✨ Thanks @graygilmore! Your snapshot has been published to npm. Built from Test the snapshot by installing your package globally: pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20261007201540Caution After installing, validate the version by running |
EvilGenius13
left a comment
There was a problem hiding this comment.
🎩 as well. Worked well!
Theme Access passwords are tied to a store's permanent .myshopify.com domain: the Theme Access proxy looks the password up by the X-Shopify-Shop header, so any other domain for the same store (for example a renamed .myshopify.com domain) fails with a 401 that reads like a wrong password. When the password is a Theme Access password, ensureAuthenticatedThemes now resolves the permanent domain from the store's public /meta.json (myshopify_domain), which needs no authentication and is served even when the storefront is password protected. Any lookup failure falls back to the domain as given, so existing setups behave as before. When a Theme Access password is still rejected, the error now says that passwords only work with the permanent domain and points at where to find it, instead of dumping the raw 401. Co-authored-by: Mbarak Bujra <mbarak.bujra@shopify.com>
The permanent-domain lookup now accepts only a bare <handle>.myshopify.com, using the existing extractMyshopifyHandle helper instead of its own regex. myshopify.io and shop.dev are dropped: the lookup never runs against a local server, and production permanent domains are always .myshopify.com. The rejected-password error no longer links to <store>/meta.json. That URL is built from the domain the user passed, so it is broken whenever that domain is not a real store. It now points to Settings > Domains in the Shopify admin. Co-authored-by: Mbarak Bujra <mbarak.bujra@shopify.com>
919065b to
efce3a1
Compare
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationspackages/cli-kit/dist/private/node/session/permanent-store-domain.d.ts/**
* Resolves the permanent `.myshopify.com` domain of a store.
*
* A store can be reached through several domains (a renamed `.myshopify.com` domain, a custom domain), but some
* Shopify services only recognise the store's permanent domain. The Theme Access app is one of them: its passwords
* are tied to the permanent domain, so a request made with any other domain fails with a 401.
*
* The permanent domain is read from the store's public `/meta.json` endpoint, which needs no authentication and is
* served even when the storefront is password protected. When the lookup fails for any reason, the domain is
* returned unchanged, so callers behave exactly as they would without this lookup.
*
* Results are cached for the lifetime of the process.
*
* @param storeFqdn - The store domain the user provided, for example `my-store.myshopify.com`.
* @returns The store's permanent domain, or `storeFqdn` when it cannot be determined.
*/
export declare function resolvePermanentStoreFqdn(storeFqdn: string): Promise<string>;
/**
* Clears the in-process cache used by {@link resolvePermanentStoreFqdn}. Intended for tests.
*/
export declare function clearPermanentStoreFqdnCache(): void;
Existing type declarationspackages/cli-kit/dist/public/node/session.d.ts@@ -121,6 +121,10 @@ export declare function ensureAuthenticatedAdmin(store: string, scopes?: AdminAP
* If a password is provided, that token will be used against Theme Access API.
* Otherwise, it will ensure that the user is authenticated with the Admin API.
*
+ * Theme Access passwords only work with the store's permanent `.myshopify.com` domain, so when the password is a
+ * Theme Access password the returned session uses the permanent domain of `store`, even if `store` is another of the
+ * store's domains.
+ *
* @param store - Store fqdn to request auth for.
* @param password - Password generated from Theme Access app.
* @param scopes - Optional array of extra scopes to authenticate with.
|
WHY are these changes introduced?
Theme Access passwords only work with a store's permanent
.myshopify.comdomain. The Theme Access proxy looks the password up by theX-Shopify-Shopheader, which the CLI fills from--store. If you pass any other domain for the same store, for example a store whose.myshopify.comname was changed (the new name is its primary domain, but the permanent domain stays something likeabc123-xy.myshopify.com), the proxy returns a 401. That 401 looks exactly like a wrong password:Nothing in the error points at the domain, so the user has no idea what to change.
WHAT is this pull request doing?
ensureAuthenticatedThemes: when the password is a Theme Access password (shptka_), it now resolves the store's permanent domain from the store's public/meta.json(myshopify_domain) and uses that asstoreFqdn./meta.jsonneeds no authentication and Core serves it even when the storefront is password protected. When the domain changes, the CLI prints one line:Using the permanent domain <permanent> for <given>.myshopify_domain, network error, 5s timeout), the domain is used exactly as given, so existing setups behave as before. Results are cached per process (theme devre-authenticates periodically). The lookup is skipped forSHOPIFY_SERVICE_ENV=local.fetchApiVersions: a 401 on a Theme Access session now explains that passwords only work on the store they were generated for and only with its permanent domain, and points to Settings > Domains in the Shopify admin to find that domain, instead of dumping the raw error.shpat_) and OAuth sessions are unchanged.How to manually test your changes?
.myshopify.comdomain differs from its permanent domain (comparehttps://<store>/meta.jsondomainvsmyshopify_domain), and generate a Theme Access password for it.shopify theme list --store <non-permanent>.myshopify.com --password <shptka_...>.Using the permanent domain <permanent> for <non-permanent>.and the theme list.shptka_password. The error should now explain the permanent-domain requirement and direct you to Settings > Domains in the Shopify admin.Checklist
patchfor bug fixes ·minorfor new features ·majorfor breaking changes) and added a changeset withpnpm changeset add