AND the OrSearchFilter's clauses against the rest of the query (#237)

[silverbackdan]

Closes #237. Found while implementing #234 (PR #236).

The bug

OrSearchFilter::addWhereByStrategy() built its clauses with $queryBuilder->orWhere(...), which ORs against the entire accumulated WHERE rather than among the filter's own clauses.

This bundle's query extensions carry no explicit priority (default 0) while API Platform's FilterExtension is -16, so the extensions add their predicates first and the filter then ORed them away:

(route.liveAt IS NOT NULL AND route.liveAt <= :now) OR (route.path LIKE :path)

A filter parameter is attacker-controlled, so the predicate being discarded is a security one. On main, anonymous GET /_/routes?path=launch returned a route scheduled for 2999.

The blast radius was wider than routes, and the issue named the wrong mechanism

The publishable case reproduces: @loginUser without draft permission, GET /component/dummy_publishable_components?reference=is returned 12 resources where 6 were expected — every draft surfaced alongside the published ones.

But not via the NOT IN (SELECT IDENTITY(o2.publishedResource) …) sub-select the issue names. That branch only runs for a caller who has draft access. The path that actually breaks is updateQueryBuilderForUnauthorizedUsers() → PublicationDate::andWhereActive() (publishedAt IS NOT NULL AND publishedAt <= :now), and that is the predicate being ORed away. The conclusion in the issue was right; its stated mechanism was the admin-path one.

The fix

All ten orWhere() call sites (exact, partial, start, end, word_start × multi-value/single-value) append to a shared accumulator; apply() is overridden to clear it, delegate to parent::apply(), then apply the whole set as one andWhere($qb->expr()->orX(...)).

The accumulator is shared across the per-field calls, not built inside addWhereByStrategy(). That method is invoked once per query parameter, so building it per call would turn multi-field search into AND and silently destroy the filter's entire purpose. It is cleared at entry and in a finally, so nothing survives a request under worker mode — pinned by test_clauses_from_one_request_do_not_leak_into_the_next.

Doctrine's DDC-1237 handling in Expr\Composite::processQueryPart() parenthesises the word_start clause (a two-LIKE string containing OR) once it is ANDed on, so precedence is safe. There is an explicit test asserting that exact DQL rather than trusting it.

Tests — failed first, for the right reason

Unit: 25 of 26 failed with DQL reading o.publishedAt IS NOT NULL **OR** … where it must be AND.
Behat: the 4 gating scenarios failed (The node 'member[0]' exists, Expected 6 resources but received 12), while the 3 OR-semantics guards passed before and after — which is what proves the fix did not turn the filter into an AND or a no-op.

• PHPUnit 678 → 704
• Behat 540 → 547

No scenario changed behaviour, and here is why each was already safe

Every filtered-collection scenario in the suite was re-read:

• /_/pages?reference=, ?title=, ?uiComponent= — all @loginAdmin, so RoutableExtension short-circuits and adds no predicate
• /_/pages?isTemplate=1 — API Platform's own SearchFilter, which already uses andWhere
• /_/layouts?reference=, ?uiComponent= — Layout is neither RoutableInterface nor #[Publishable]
• /_/routes?itemsPerPage=10 — not a filter property, so OrSearchFilter never runs
• ?published=true|false — read by PublishableExtension from the filter context, not an OrSearchFilter property

The one scenario that had been passing for the wrong reason was already rewritten in #236.

Two findings recorded

DummyOrSearchFilterable had zero coverage of any kind — no unit test, no scenario — despite existing as a dedicated fixture. That absence is the direct reason this survived. It now has both.

The five single-value branches are unreachable from filterProperty(): normalizeValues((array) $value, $property) always hands over an array, which is why a "single value" still generates the array-suffixed parameter name :field1_p10. They are fixed anyway since the method is protected, but the class is final, so they are dead code and a candidate for collapsing.

Behavioural change for upgraders

Filtered collections will return fewer rows wherever an extension predicate was previously being discarded. That is the point of the fix, but it warrants a release note. No migration, no config change.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 88.33%. Comparing base (12d5f2f) to head (94149b7).

Files with missing lines	Patch %	Lines
src/Filter/OrSearchFilter.php	94.11%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #238      +/-   ##
============================================
+ Coverage     87.81%   88.33%   +0.51%     
+ Complexity     2665     2657       -8     
============================================
  Files           258      258              
  Lines          7730     7695      -35     
============================================
+ Hits           6788     6797       +9     
+ Misses          942      898      -44     

Flag	Coverage Δ
behat	69.21% <76.47%> (+0.36%)	⬆️
phpunit	35.92% <94.11%> (+0.83%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.