Skip to content

Configure request headers used in cache keys - #800

Open
maxschmeling wants to merge 17 commits into
trunkfrom
codex/filter-cache-invalidating-headers
Open

Configure request headers used in cache keys#800
maxschmeling wants to merge 17 commits into
trunkfrom
codex/filter-cache-invalidating-headers

Conversation

@maxschmeling

@maxschmeling maxschmeling commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add query-level cache_key_request_headers configuration for HTTP queries
  • always retain Authorization and Cache-Control, merging additional query header names case-insensitively
  • make get_cache_key_request_headers() part of HttpQueryInterface and have QueryRunner consume the query-owned policy directly
  • carry the resolved list in one internal request header used consistently by cache lookup, storage, and logging
  • strip the internal metadata before outbound dispatch and before storing the request in the cache entry
  • configure Shopify’s built-in queries to include the storefront access-token header
  • document the cache-isolation security risk, query-only configuration, and the direct-interface implementation requirement

Security

Remote Data Blocks does not include every request header in cache keys. If a custom header changes authentication, authorization, tenancy, or returned data, every query that uses it must list that header in cache_key_request_headers; otherwise requests with different security contexts can share a cached response.

Headers configured on a data source are not added automatically. The documentation calls this out prominently, including the cross-request and cross-user risk with persistent object caching. The built-in Authorization and Cache-Control defaults cannot be removed.

Compatibility

Direct implementations of HttpQueryInterface must implement get_cache_key_request_headers(). Returning [] uses only the built-in defaults. The extending guide includes an upgrade example.

Testing

  • npm run format:check
  • npm run lint:php
  • npm run lint:js
  • npm run check-types
  • npm test (596 PHP tests / 1,374 assertions; 76 JavaScript tests)
  • npm run build
  • git diff --check

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Test this PR in WordPress Playground.

@maxschmeling maxschmeling changed the title Add filter for cache-invalidating request headers Configure request headers used in cache keys Aug 18, 2026
Comment thread inc/Config/DataSource/HttpDataSource.php Outdated
Comment thread inc/Config/CacheKeyRequestHeadersInterface.php Outdated
Comment thread inc/HttpClient/RdbCacheStrategy.php Outdated
Comment thread inc/HttpClient/RdbCacheMiddleware.php
Comment thread inc/Config/Query/HttpQuery.php Outdated
Comment thread inc/HttpClient/CacheKeyRequestHeaders.php Outdated
Co-authored-by: Chris Zarate <chris.zarate@automattic.com>
Comment thread inc/HttpClient/CacheKeyRequestHeaders.php Outdated
Comment thread inc/HttpClient/RdbCacheMiddleware.php
parent::__construct(
$storage ?? new WordPressObjectCacheStorage( self::WP_OBJECT_CACHE_GROUP ),
self::FALLBACK_CACHE_TTL_IN_SECONDS,
$vary_headers

@chriszarate chriszarate Aug 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Cache poisoning is still possible in the future. The vary headers passed to the constructor ([]) and the ones used in get_object_cache_key_from_request are different. That is why I suggested porting the code from GreedyCacheStrategy and no longer extending it. I don't think we can honor its contract.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, you're right, it's fine now but not safe for the future.

Copied the implementation here 1cde10a

Small tweak here: 9ae5731

Testing and checking now

Comment thread inc/HttpClient/RdbCacheMiddleware.php Outdated
Comment thread inc/HttpClient/RdbLogMiddleware.php Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants