fix: use cacheable GET reads only for fresh default guest sessions - #2720
Maciek Kucmus (mkucmus) wants to merge 8 commits into
Conversation
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-package session, cart, SSR, and shared-cache behavior warrants final human validation.
Pull request overview
Centralizes safe cacheable Store API reads, addressing #2692 while preventing personalized sessions from consuming shared guest responses.
Changes:
- Adds
useCacheableRead, route classification, migrations, and coverage tests. - Adds case-insensitive header merging and anonymous-response token protection.
- Shares cart state app-wide and documents guest SSR behavior.
File summaries
| File | Description |
|---|---|
templates/vue-starter-template/README.md |
Documents safe cacheable reads. |
templates/vue-starter-template/app/components/form/SalutationSelect.vue |
Uses centralized read routing. |
templates/vue-starter-template/app/components/form/CountrySearchSelect.vue |
Uses centralized country reads. |
templates/vue-demo-store/app/pages/wishlist.vue |
Migrates wishlist product reads. |
pnpm-lock.yaml |
Removes defu dependency. |
packages/nuxt-module/plugin.ts |
Provides cart and guest SSR state. |
packages/nuxt-module/plugin.test.ts |
Tests new plugin state. |
packages/composables/src/useUser/useUser.ts |
Migrates reference-data reads. |
packages/composables/src/useUser/useUser.test.ts |
Tests guest and logged-in routing. |
packages/composables/src/useShopwareContext/useShopwareContext.ts |
Exposes guest SSR state. |
packages/composables/src/useSalutations/useSalutations.ts |
Centralizes salutation reads. |
packages/composables/src/useSalutations/useSalutations.spec.ts |
Updates salutation assertions. |
packages/composables/src/useProductSearch/useProductSearch.ts |
Centralizes product-detail reads. |
packages/composables/src/useProductSearch/useProductSearch.test.ts |
Tests currency-sensitive routing. |
packages/composables/src/useProductReviews/useProductReviews.ts |
Centralizes review reads. |
packages/composables/src/useProductReviews/useProductReviews.test.ts |
Updates review assertions. |
packages/composables/src/useProductConfigurator/useProductConfigurator.ts |
Centralizes variant reads. |
packages/composables/src/useProductConfigurator/useProductConfigurator.spec.ts |
Updates variant assertions. |
packages/composables/src/useNavigationSearch/useNavigationSearch.ts |
Centralizes SEO URL reads. |
packages/composables/src/useNavigationSearch/useNavigationSearch.test.ts |
Updates SEO read assertions. |
packages/composables/src/useNavigation/useNavigation.ts |
Centralizes navigation reads. |
packages/composables/src/useNavigation/useNavigation.test.ts |
Verifies GET query conversion. |
packages/composables/src/useInternationalization/useInternationalization.ts |
Centralizes language reads. |
packages/composables/src/useInternationalization/useInternationalization.test.ts |
Updates language assertions. |
packages/composables/src/useCountries/useCountries.ts |
Centralizes country reads. |
packages/composables/src/useCountries/useCountries.test.ts |
Updates country assertions. |
packages/composables/src/useCategorySearch/useCategorySearch.ts |
Centralizes category reads. |
packages/composables/src/useCategorySearch/useCategorySearch.test.ts |
Updates category assertions. |
packages/composables/src/useCacheableRead/useCacheableRead.ts |
Implements safe GET selection. |
packages/composables/src/useCacheableRead/useCacheableRead.test.ts |
Covers session and cart cases. |
packages/composables/src/useCacheableRead/cacheableReadCoverage.test.ts |
Enforces route classification. |
packages/composables/src/index.ts |
Exports the new composable. |
packages/composables/src/createShopwareContext.ts |
Adds guest SSR configuration. |
packages/composables/src/createShopwareContext.test.ts |
Tests configuration defaults. |
packages/composables/src/_test/useSetup.ts |
Supports mocked default headers. |
packages/cms-base-layer/app/components/public/cms/element/CmsElementProductDescriptionReviews.vue |
Migrates CMS review reads. |
packages/api-client/src/resolveRequestHeaders.ts |
Uses normalized header merging. |
packages/api-client/src/resolveRequestHeaders.test.ts |
Tests empty-header removal. |
packages/api-client/src/helpers/mergeRequestHeaders.ts |
Adds header merge helper. |
packages/api-client/src/helpers/mergeRequestHeaders.test.ts |
Tests merge semantics. |
packages/api-client/src/helpers/index.ts |
Exports header helper. |
packages/api-client/src/createAPIClient.ts |
Prevents anonymous token adoption. |
packages/api-client/src/createApiClient.test.ts |
Covers token and concurrency behavior. |
packages/api-client/README.md |
Documents header APIs. |
packages/api-client/package.json |
Removes defu. |
apps/docs/src/best-practices/caching.md |
Revises caching guidance. |
AGENTS.md |
Updates contributor conventions. |
.changeset/shared-carts-stay.md |
Records Nuxt module changes. |
.changeset/quiet-tokens-stay.md |
Records API client changes. |
.changeset/fresh-guests-read.md |
Records composables changes. |
.changeset/calm-reviews-read.md |
Records CMS layer changes. |
Review details
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
- Files reviewed: 50/51 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Stale comment
Security review
No medium, high, or critical vulnerabilities in this change set.
There were no prior automation security-finding threads to re-validate.
Traced the new
invokeRead/canUseCacheableGetgate, header merge (""removessw-context-tokencase-insensitively), andonResponsetoken adoption. Cacheable GETs are limited to fresh default-guest sessions, go out without a context token, and cannot replace the client token from an anonymous orCache-Control: publicresponse (including a login that finishes during the read). The latest commit fail-closes missing session ids and a cart without alineItemsarray. Residual stale-state cases fail toward default guest data, not toward putting another session into the shared cache. No new dependencies.Sent by Cursor Automation: Review pull requests for exploitable security issues and flag only validated findings before merge
There was a problem hiding this comment.
Stale comment
Security review
No medium, high, or critical vulnerabilities in this change set.
There were no prior automation security-finding threads to re-validate.
Re-reviewed after
567c19a2(docs plus header-drop hardening). Cacheable GETs still require a fresh default-guest session, go out withsw-context-tokenremoved, and cannot adopt a token from an anonymous orCache-Control: publicresponse.mergeRequestHeadersnow deletes a header in every casing when any caller key is"", so a mixed-case token cannot survive the strip. Missing session ids and a cart withoutlineItemsstill fail closed. No new dependencies.Sent by Cursor Automation: Review pull requests for exploitable security issues and flag only validated findings before merge
There was a problem hiding this comment.
Security review
No medium, high, or critical vulnerabilities in this change set.
There were no prior automation security-finding threads to re-validate.
Re-reviewed after 74077f3d (test-only: coverage helper now matches quoted operation keys with includes instead of RegExp). Production controls are unchanged: cacheable GETs still require a fresh default-guest session, go out with sw-context-token removed, and cannot adopt a token from an anonymous or Cache-Control: public response. mergeRequestHeaders still deletes a header in every casing when any caller key is "". Missing session ids and a cart without a lineItems array still fail closed. No new dependencies.
Sent by Cursor Automation: Review pull requests for exploitable security issues and flag only validated findings before merge


Problem
With
cacheableReadson, every session read catalog data over GET. Shared caches key on the URL, not onsw-context-token. So a logged-in, cart or currency-switched session could get the cached guest response. On the starter, a €3,417.11 product showed €4,000.92 after a currency switch. Cached responses also carried the first visitor's token. Nothing checked that read composables honour the flag.Fix
@shopware/composables: newuseCacheableRead().invokeRead(postOperation, params). It sends the GET twin only when the loaded session and cart look like a fresh default guest. That GET has nosw-context-token. Every other session keeps the POST. All flag branches use it.useShopwareContext().cacheableReadsis deprecated for branching.@shopware/api-client: newmergeRequestHeaders. Header names match case-insensitively. A header set to""is removed. A request that drops the client token never adopts one from its response.defudependency removed.@shopware/nuxt-module: app-wideswCartref, plusguestServerRenderfor server renders without a visitor session.invokeRead.Closes #2692
Why
invokeReadand not per-call branchesThe decision needs the session, the cart and the token. Written by hand, that is 16 copies. They already drifted:
useListingnever got a branch (#2691),useProductConfiguratorputincludeswhere GET ignores it. One helper, tested once. The trade-off: the code names the POST operation while the network may show GET.Verification
Not in this PR
useListing(useListing ignores cacheableReads, so listings and search stay on POST #2691),useLandingSearch,useProductAssociations, and the starter's currency and search-suggest reads stay POST. Listed in the coverage test.sw-language-id. Can be relaxed after shopware#20200 ships.