Skip to content

Force strict equivalence in mongo role checks #834

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
abnegate merged 1 commit into from
Mar 12, 2026
Merged

Force strict equivalence in mongo role checks #834

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Database/Adapter/Mongo.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2026,7 +2026,7 @@ public function find(Document $collection, array $queries = [], ?int $limit = 25
// permissions
if ($this->authorization->getStatus()) {
$roles = \implode('|', $this->authorization->getRoles());
$filters['_permissions']['$in'] = [new Regex("{$forPermission}\\(\".*(?:{$roles}).*\"\\)", 'i')];
$filters['_permissions']['$in'] = [new Regex("{$forPermission}\\(\"(?:{$roles})\"\\)", 'i')];
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 12, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's check the exact code at the three mentioned lines
sed -n '2025,2035p' src/Database/Adapter/Mongo.php

Repository: utopia-php/database

Length of output: 1882

🏁 Script executed:

# Check lines 2290 and 2380
sed -n '2285,2295p' src/Database/Adapter/Mongo.php
sed -n '2375,2385p' src/Database/Adapter/Mongo.php

Repository: utopia-php/database

Length of output: 1882

🏁 Script executed:

# Check the Permission and Role helper classes to understand toString() behavior
cat src/Database/Helpers/Permission.php

Repository: utopia-php/database

Length of output: 7107

🏁 Script executed:

# Check Role.php
cat src/Database/Helpers/Role.php

Repository: utopia-php/database

Length of output: 4142

🏁 Script executed:

# Search for how getRoles() is implemented to understand what values it returns
rg -A 5 "function getRoles" src/

Repository: utopia-php/database

Length of output: 409

Escape role values before interpolating them into these permission regexes.

This is not a strict-equivalence check. Permission::toString() stores permission("role") strings where the role contains identifiers and dimensions appended verbatim (e.g., user:identifier/dimension). Since getRoles() returns raw role strings and the regex is built via implode('|', $this->authorization->getRoles()) without escaping, any regex metacharacters in identifiers or dimensions will be interpreted as regex syntax rather than literal characters. This causes over-matching or under-matching in find(), count(), and sum().

🔧 Suggested fix 
-            $roles = \implode('|', $this->authorization->getRoles());
-            $filters['_permissions']['$in'] = [new Regex("{$forPermission}\\(\"(?:{$roles})\"\\)", 'i')];
+            $roles = \implode('|', \array_map(
+                static fn (string $role): string => \preg_quote($role, '/'),
+                $this->authorization->getRoles()
+            ));
+            $permission = \preg_quote($forPermission, '/');
+            $filters['_permissions']['$in'] = [new Regex("{$permission}\\(\"(?:{$roles})\"\\)", 'i')];

Apply the same escaping in the count() and sum() branches as well.

Also applies to: 2290–2290, 2380–2380

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/Database/Adapter/Mongo.php` at line 2029, The permission regex is built
from raw role strings so metacharacters in roles can break matching; update the
code that builds $roles (used in new
Regex("{$forPermission}\\(\"(?:{$roles})\"\\)", 'i')) to escape each role with
preg_quote (specifying the regex delimiter, e.g., '/') before imploding with '|'
(e.g., $roles = implode('|', array_map(fn($r) => preg_quote($r, '/'),
$this->authorization->getRoles()))), and apply the same change in the count()
and sum() branches where the identical Regex construction occurs so all three
usages (find/count/sum) use escaped role values.

}

$options = [];
Expand Down Expand Up @@ -2287,7 +2287,7 @@ public function count(Document $collection, array $queries = [], ?int $max = nul
// Add permissions filter if authorization is enabled
if ($this->authorization->getStatus()) {
$roles = \implode('|', $this->authorization->getRoles());
$filters['_permissions']['$in'] = [new Regex("read\\(\".*(?:{$roles}).*\"\\)", 'i')];
$filters['_permissions']['$in'] = [new Regex("read\\(\"(?:{$roles})\"\\)", 'i')];
}

/**
Expand Down Expand Up @@ -2377,7 +2377,7 @@ public function sum(Document $collection, string $attribute, array $queries = []
// permissions
if ($this->authorization->getStatus()) { // skip if authorization is disabled
$roles = \implode('|', $this->authorization->getRoles());
$filters['_permissions']['$in'] = [new Regex("read\\(\".*(?:{$roles}).*\"\\)", 'i')];
$filters['_permissions']['$in'] = [new Regex("read\\(\"(?:{$roles})\"\\)", 'i')];
}

// using aggregation to get sum an attribute as described in
Expand Down
Loading