[rebsweb]

Responses with a positive X-Accel-Expires value currently never enter the FastCGI cache: the generator passes the header directly to fastcgi_no_cache, where any nonzero value forbids storage.

Remove that argument and let Nginx apply the header's native TTL semantics. Add an explicit response Cache-Control guard for private, no-cache and no-store, because a positive X-Accel-Expires otherwise takes precedence over those native checks. A zero value still disables caching, and the existing skip/cookie protections remain. Update the relevant assertions in the existing generator test; no new test infrastructure, workflows or DevOps files.

Local validation: the updated test failed before the fix. All 115 PHPUnit tests (571 assertions) passed in the existing PHP 8.5 container with Sodium. Real isolated Nginx/FPM requests using freshly generated configuration confirmed normal and positive-60-second responses go MISS → HIT, while zero TTL, private/no-store responses and Set-Cookie remain MISS → MISS. The same protection was verified with positive TTL combined with private/no-store, no-cache and cookies, including mixed-case directives, qualified private values and repeated Cache-Control headers. Coding standards, hook documentation and static analysis passed. Host composer qa encountered a missing Sodium extension; its full suite passed in the container with that extension.

Consumers need to regenerate both HTTP and FastCGI Nginx includes together after installing the fixed plugin release, because the FastCGI include references the new response-header map. This PR does not change deployed configuration or run cache purges.

[brianvarskonst]

Verdict: The logic is correct; two small things should land before merging. CI is green, including "Native Nginx config integration", so nginx -t accepts the new regex.

What's right

• Root cause is correct: fastcgi_no_cache treats any value other than empty or "0" as "don't cache", so a positive X-Accel-Expires has blocked caching entirely.
• The new guard is needed: a positive X-Accel-Expires overrides Nginx's native Cache-Control: private/no-store/no-cache checks.
• I ran the regex against sample values. It matches private, Private, public,no-store, public , private, no-cache="Set-Cookie" and WordPress' 404 header. It does not match x-private, no-store-foo, max-age=0, public, max-age=60 (the NotFoundCacheSubscriber value).
• ResponseCacheability already only blocks on x-accel-expires:0, so Nginx now behaves the way the diagnostics already report.

Findings

1. Medium – Missing CHANGELOG entry with an upgrade note. Fix PRs so far carry their own CHANGELOG entry (#48, 738969d). It matters here: anyone who regenerates only the FastCGI include gets unknown "sympress_cache_skip_cache_control" variable from nginx -t. That fails loudly rather than silently, but it still breaks the deploy. The "regenerate the HTTP and FastCGI includes together" note currently lives only in the PR description.

2. Medium – No test checks the behavior of the regex that protects private responses. The unit test only asserts that the map $upstream_http_cache_control … line exists. The Nginx integration test only validates syntax, because its fixture has no upstream and $upstream_http_cache_control is always empty there. If the regex regresses later, private responses get cached and nothing notices; the PR description only lists manual Nginx/FPM checks. Cheap fix: in NginxConfigGeneratorTest, assert the exact regex line and run a small preg_match table against the same pattern with the cases above.

3. Low / question – Repeated Cache-Control headers on Nginx < 1.23. As far as I know, $upstream_http_* only returns the first header there; combining repeated headers came with 1.23. On Nginx 1.22 (Debian 12) with a positive X-Accel-Expires, a private in a second header could be missed. Repeated headers are rare from PHP, but the README documents no minimum Nginx version. Either document one or explicitly accept the residual risk.

Not verified: behavior in a real Nginx/FPM setup (relying on the checks listed in the PR description); the Nginx version detail in point 3 is from memory. After merging, pages that previously sent a positive X-Accel-Expires will be cached for that TTL — intended, but it may be noticeable on production sites.