Skip to content

Commit e116df0

Browse files
roxblnfkclaude
andcommitted
Add wrapWhere option to QueryScope
`QueryScope` adds its conditions at the top level of the query, where a user-supplied `orWhere` can bypass the scope due to AND-over-OR precedence (e.g. a `status = active` scope leaks on `where(id,1)->orWhere(id,2)`). Add a `wrapWhere` constructor flag that calls `QueryBuilder::wrapWhere()` before applying the scope conditions, enclosing already-registered user conditions in a parenthesized AND-group. For scopes on joined loaders the call is forwarded to the JOIN ON tokens via `wrapOnWhere`. The flag defaults to false to preserve backwards-compatible behavior. - QueryScope: new `wrapWhere` parameter with a thorough docblock - Unit tests for QueryScope apply/wrap behavior, including the empty-where edge case - HasMany integration tests switched from the throwaway WrappedQueryScope fixture to `QueryScope(wrapWhere: true)`, exercising the real class; the fixture is removed Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent cd4ea0f commit e116df0

4 files changed

Lines changed: 141 additions & 36 deletions

File tree

src/Select/QueryScope.php

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,62 @@
55
namespace Cycle\ORM\Select;
66

77
/**
8-
* Provides the ability to scope query and load necessary relations into the loader.
8+
* A ready-made {@see ScopeInterface} implementation that applies a set of WHERE
9+
* conditions and an ORDER BY rule to the query.
10+
*
11+
* // Only active records, newest first
12+
* new QueryScope(['status' => 'active'], ['created_at' => 'DESC']);
13+
*
14+
* ## Protecting the scope from `orWhere` bypass
15+
*
16+
* By default the conditions are added at the top level of the query. Because SQL
17+
* binds AND tighter than OR, a user-supplied `orWhere` can bypass the scope:
18+
*
19+
* // scope: WHERE status = 'active'
20+
* $select->scope(new QueryScope(['status' => 'active']))
21+
* ->where('id', 1)
22+
* ->orWhere('id', 2);
23+
* // WHERE id = 1 OR id = 2 AND status = 'active'
24+
* // ≡ WHERE id = 1 OR (id = 2 AND status = 'active') ← scope leaks on first arm
25+
*
26+
* Pass `wrapWhere: true` to enclose the already-registered conditions in a
27+
* parenthesized group before adding the scope's own, keeping it effective
28+
* regardless of how user code mixes AND/OR:
29+
*
30+
* $select->scope(new QueryScope(['status' => 'active'], wrapWhere: true))
31+
* ->where('id', 1)
32+
* ->orWhere('id', 2);
33+
* // WHERE (id = 1 OR id = 2) AND status = 'active'
34+
*
35+
* The wrap is applied via {@see QueryBuilder::wrapWhere()}, which is a no-op when
36+
* no conditions have been registered yet. For scopes attached to joined relation
37+
* loaders the call is automatically forwarded to the JOIN's ON tokens. Passing an
38+
* empty `$where` together with `wrapWhere: true` is still meaningful — it protects
39+
* user conditions without adding any of its own.
40+
*
41+
* See {@see ScopeInterface} for the full discussion of scope bypass.
942
*/
1043
final class QueryScope implements ScopeInterface
1144
{
45+
/**
46+
* @param array $where Conditions to apply, in {@see QueryBuilder::where()} array syntax.
47+
* @param array $orderBy ORDER BY rules, in {@see QueryBuilder::orderBy()} array syntax.
48+
* @param bool $wrapWhere When true, wrap already-registered conditions in a nested
49+
* AND-group before adding this scope's conditions (protects against `orWhere`
50+
* bypass). Defaults to false to preserve backwards-compatible behavior.
51+
*/
1252
public function __construct(
13-
private array $where,
14-
private array $orderBy = [],
53+
private readonly array $where,
54+
private readonly array $orderBy = [],
55+
private readonly bool $wrapWhere = false,
1556
) {}
1657

1758
public function apply(QueryBuilder $query): void
1859
{
60+
if ($this->wrapWhere) {
61+
$query->wrapWhere();
62+
}
63+
1964
$query->where($this->where)->orderBy($this->orderBy);
2065
}
2166
}

tests/ORM/Fixtures/WrappedQueryScope.php

Lines changed: 0 additions & 29 deletions
This file was deleted.

tests/ORM/Functional/Driver/Common/Relation/HasMany/HasManyScopeTest.php

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
use Cycle\ORM\Tests\Fixtures\Comment;
1616
use Cycle\ORM\Tests\Fixtures\SortByIDScope;
1717
use Cycle\ORM\Tests\Fixtures\User;
18-
use Cycle\ORM\Tests\Fixtures\WrappedQueryScope;
1918
use Cycle\ORM\Tests\Traits\TableTrait;
2019
use Cycle\Database\Exception\StatementException;
2120

@@ -485,14 +484,14 @@ public function testInloadJoinedScopeWithoutWrapWhereIsBypassedByAdversarialOrLo
485484
}
486485

487486
/**
488-
* Companion to the test above: with {@see WrappedQueryScope} (calls wrapWhere() first),
487+
* Companion to the test above: with `QueryScope(..., wrapWhere: true)`,
489488
* QueryBuilder::targetFunc() forwards the wrap to `wrapOnWhere` on the JOIN's ON tokens,
490489
* enclosing the user's OR group. The scope's `level >= 2` condition stays effective.
491490
*/
492491
public function testInloadJoinedScopeWithWrapWhereProtectsAgainstAdversarialOrLoad(): void
493492
{
494493
$this->orm = $this->withCommentsSchema([
495-
Schema::SCOPE => new WrappedQueryScope(['@.level' => ['>=' => 2]]),
494+
Schema::SCOPE => new Select\QueryScope(['@.level' => ['>=' => 2]], wrapWhere: true),
496495
]);
497496

498497
$res = (new Select($this->orm, User::class))
@@ -525,7 +524,7 @@ public function testInloadJoinedScopeWithWrapWhereProtectsAgainstAdversarialOrLo
525524
public function testInloadJoinedScopeWithWrapWhereWithoutAdversarialLoad(): void
526525
{
527526
$this->orm = $this->withCommentsSchema([
528-
Schema::SCOPE => new WrappedQueryScope(['@.level' => ['>=' => 3]]),
527+
Schema::SCOPE => new Select\QueryScope(['@.level' => ['>=' => 3]], wrapWhere: true),
529528
]);
530529

531530
$res = (new Select($this->orm, User::class))
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Cycle\ORM\Tests\Unit\Select;
6+
7+
use Cycle\Database\Query\SelectQuery;
8+
use Cycle\ORM\Select\AbstractLoader;
9+
use Cycle\ORM\Select\QueryBuilder;
10+
use Cycle\ORM\Select\QueryScope;
11+
use PHPUnit\Framework\TestCase;
12+
13+
final class QueryScopeTest extends TestCase
14+
{
15+
public function testAppliesWhereAndOrderBy(): void
16+
{
17+
$query = new SelectQuery(['users']);
18+
$scope = new QueryScope(['status' => 'active'], ['created_at' => 'DESC']);
19+
20+
$scope->apply($this->makeBuilder($query));
21+
22+
$tokens = $query->getTokens();
23+
$this->assertCount(1, $tokens['where']);
24+
$this->assertSame('users.status', $tokens['where'][0][1][0]);
25+
$this->assertSame([['users.created_at', 'DESC']], $tokens['orderBy']);
26+
}
27+
28+
public function testDoesNotWrapByDefault(): void
29+
{
30+
$query = (new SelectQuery(['users']))
31+
->where('id', 1)
32+
->orWhere('id', 2);
33+
34+
(new QueryScope(['status' => 'active']))->apply($this->makeBuilder($query));
35+
36+
// Flat token list: no opening-paren group inserted before the user tokens.
37+
$where = $query->getTokens()['where'];
38+
$this->assertNotSame(['AND', '('], $where[0]);
39+
// Scope condition appended at the top level.
40+
$this->assertSame('users.status', $where[\count($where) - 1][1][0]);
41+
}
42+
43+
public function testWrapWhereEnclosesExistingConditions(): void
44+
{
45+
$query = (new SelectQuery(['users']))
46+
->where('id', 1)
47+
->orWhere('id', 2);
48+
49+
(new QueryScope(['status' => 'active'], wrapWhere: true))->apply($this->makeBuilder($query));
50+
51+
$where = $query->getTokens()['where'];
52+
53+
// User conditions are now enclosed in a leading AND-group...
54+
$this->assertSame(['AND', '('], $where[0]);
55+
$this->assertSame(['', ')'], $where[3]);
56+
// ...with the scope condition appended after the group.
57+
$this->assertSame('users.status', $where[4][1][0]);
58+
}
59+
60+
public function testWrapWhereWithEmptyConditionsStillProtectsUserWhere(): void
61+
{
62+
$query = (new SelectQuery(['users']))
63+
->where('id', 1)
64+
->orWhere('id', 2);
65+
66+
// Empty $where but wrapWhere: true — should wrap user conditions, add nothing.
67+
(new QueryScope([], wrapWhere: true))->apply($this->makeBuilder($query));
68+
69+
$this->assertEquals(
70+
[
71+
['AND', '('],
72+
['AND', ['id', '=', $query->getTokens()['where'][1][1][2]]],
73+
['OR', ['id', '=', $query->getTokens()['where'][2][1][2]]],
74+
['', ')'],
75+
],
76+
$query->getTokens()['where'],
77+
);
78+
}
79+
80+
private function makeBuilder(SelectQuery $query): QueryBuilder
81+
{
82+
$loader = $this->createMock(AbstractLoader::class);
83+
$loader->method('fieldAlias')->willReturn(null);
84+
$loader->method('getAlias')->willReturn('users');
85+
$loader->method('getTarget')->willReturn('user');
86+
$loader->method('getParentLoader')->willReturn(null);
87+
88+
return new QueryBuilder($query, $loader);
89+
}
90+
}

0 commit comments

Comments
 (0)