Skip to content

fix: skip Sequence validation for $tenant attribute #838

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

Open
abnegate wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: skip Sequence validation for $tenant attribute #838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
280432a
fix: skip Sequence validation for $tenant attribute
abnegate Mar 14, 2026
372e622
fix: use loose comparison for tenant checks to handle int/string mism…
abnegate Mar 14, 2026
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
12 changes: 6 additions & 6 deletions src/Database/Database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1856,7 +1856,7 @@ public function updateCollection(string $id, array $permissions, bool $documentS

if (
$this->adapter->getSharedTables()
&& $collection->getTenant() !== $this->adapter->getTenant()
&& $collection->getTenant() != $this->adapter->getTenant()
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 14, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify all loose tenant comparisons that should be replaced by a dedicated helper.
rg -nP 'getTenant\(\)\s*!=|!=\s*\$this->adapter->getTenant\(\)' src/Database/Database.php

Repository: utopia-php/database

Length of output: 703

🌐 Web query:

In PHP 8.x, what are the loose-comparison results for: '01' == '1', '1e3' == '1000', null == 0, and null == ''?

💡 Result:

  • '01' == '1' → true (both are numeric strings, so comparison is numeric) [1]
  • '1e3' == '1000' → true ('1e3' is a numeric string in scientific notation; numeric-string comparisons are numeric) [1]
  • null == 0 → true [2]
  • null == '' → true [2]

Sources: [1] PHP Manual — Comparison Operators (numeric-string rules; examples like "1" == "01" and "10" == "1e1") [2] PHP Manual — Type comparison tables (loose == matrix)

🏁 Script executed:

#!/bin/bash
# Get context around each loose tenant comparison
for line in 1859 1895 1950 1976 2010 7204; do
  echo "=== Line $line ==="
  sed -n "$((line-2)),$((line+2))p" src/Database/Database.php
  echo ""
done

Repository: utopia-php/database

Length of output: 1526

🏁 Script executed:

#!/bin/bash
# Check the type signatures of getTenant/setTenant methods
rg -A 3 'public function (get|set)Tenant' src/Database/Database.php | head -30

Repository: utopia-php/database

Length of output: 533

🏁 Script executed:

#!/bin/bash
# Check if there are any tests that verify tenant isolation or comparison behavior
fd -t f '\.php$' tests/ | xargs rg -l 'getTenant.*!=|tenant.*comparison' | head -5

Repository: utopia-php/database

Length of output: 45

🏁 Script executed:

#!/bin/bash
# Search for tests that verify tenant isolation or multi-tenant behavior
fd -t f '\.php$' tests/ -x rg -l 'tenant' {} \; | head -10

Repository: utopia-php/database

Length of output: 192

🏁 Script executed:

#!/bin/bash
# Check specific test files for tenant-related assertions
rg -i 'tenant.*==|tenant.*!=|shared.*table' tests/ --type php | head -20

Repository: utopia-php/database

Length of output: 1861

🏁 Script executed:

#!/bin/bash
# Verify the exact type behavior in PHP by checking if tenant conversion is already done elsewhere
rg -B 5 -A 5 'isSame|sameTenant|tenant.*===|tenant.*strict' src/Database/Database.php

Repository: utopia-php/database

Length of output: 45

🏁 Script executed:

#!/bin/bash
# Check if Document::getTenant and Collection::getTenant have same type signatures
rg -A 3 'class Document|class Collection' src/Database/ -x | grep -A 3 'getTenant'

Repository: utopia-php/database

Length of output: 45

🏁 Script executed:

#!/bin/bash
# Look at the testSharedTables test to see how tenant comparisons are expected to work
rg -A 30 'public function testSharedTables' tests/e2e/Adapter/Scopes/CollectionTests.php

Repository: utopia-php/database

Length of output: 2102

🏁 Script executed:

#!/by/bash
# Check Document class definition to see getTenant signature
rg -A 2 'class Document' src/Database/Document.php | head -10

Repository: utopia-php/database

Length of output: 144

🏁 Script executed:

#!/bin/bash
# Check Collection class definition to see getTenant signature
rg -A 2 'class Collection' src/Database/Collection.php | head -10

Repository: utopia-php/database

Length of output: 130

🏁 Script executed:

#!/bin/bash
# Search for any tenant value normalization or validation before comparison
rg -B 5 -A 5 'setTenant|getTenant' src/Database/Database.php | grep -A 5 -B 5 'int|string'

Repository: utopia-php/database

Length of output: 824

🏁 Script executed:

#!/bin/bash
# Check if there are any real-world test cases that would demonstrate the loose comparison issue
rg -B 3 -A 10 "getTenant\(\)" tests/e2e/Adapter/SharedTables/ | grep -A 10 "getTenant" | head -40

Repository: utopia-php/database

Length of output: 1534

🏁 Script executed:

#!/bin/bash
# Check the Adapter class to see if there's any tenant normalization
rg -A 5 'setTenant.*{' src/Database/Adapter.php | head -20

Repository: utopia-php/database

Length of output: 150

🏁 Script executed:

#!/bin/bash
# Search for any practical use of tenant with string values (numeric-like or otherwise)
rg 'setTenant\(' tests/e2e/Adapter/SharedTables/ | head -20

Repository: utopia-php/database

Length of output: 440

🏁 Script executed:

#!/bin/bash
# Check if Collection has a getTenant method (to understand what it returns)
fd -t f '\.php$' src/Database/ -x grep -l 'class Collection'

Repository: utopia-php/database

Length of output: 45

🏁 Script executed:

#!/bin/bash
# Look for the Collection class in the src/Database directory
find src/Database -name '*.php' -exec grep -l 'class Collection' {} \;

Repository: utopia-php/database

Length of output: 45

🏁 Script executed:

#!/bin/bash
# Check if there are any comparisons using strict equality for tenant elsewhere
rg 'getTenant.*===' src/Database/

Repository: utopia-php/database

Length of output: 313

🏁 Script executed:

#!/bin/bash
# Find Collection class definition
rg -A 10 'class Collection extends' src/Database/

Repository: utopia-php/database

Length of output: 45

🏁 Script executed:

#!/bin/bash
# Check Adapter setTenant/getTenant to see if tenant normalization happens there
rg -B 2 -A 8 'private.*\$tenant|protected.*\$tenant' src/Database/Adapter.php | head -40

Repository: utopia-php/database

Length of output: 321

🏁 Script executed:

#!/bin/bash
# See how tenant is used in actual database operations to understand if loose comparison is problematic
rg -B 3 -A 3 'getTenant.*!=.*getTenant' src/Database/Database.php

Repository: utopia-php/database

Length of output: 1765

🏁 Script executed:

#!/bin/bash
# Check if tenant values from API calls are normalized before use
rg 'cast.*tenant|intval.*tenant|strval.*tenant' src/Database/ -i

Repository: utopia-php/database

Length of output: 45

🏁 Script executed:

#!/bin/bash
# Find where Collection class is defined and check its getTenant method
rg -A 30 'class Collection' src/Database/ | head -50

Repository: utopia-php/database

Length of output: 45

🏁 Script executed:

#!/bin/bash
# Check if there are integration tests that specifically test tenant isolation with mixed types
rg -B 5 -A 15 'setTenant.*[0-9]' tests/e2e/Adapter/SharedTables/*.php | grep -A 15 'setTenant' | head -60

Repository: utopia-php/database

Length of output: 4249

🏁 Script executed:

#!/bin/bash
# Verify if Collection stores its own tenant or gets it from Database context
rg 'Collection.*tenant|new Collection' src/Database/Database.php -B 3 -A 3 | head -50

Repository: utopia-php/database

Length of output: 45

🏁 Script executed:

#!/bin/bash
# Search for what getCollection returns and its actual class
rg 'function getCollection|->getCollection' src/Database/Database.php -A 3 | head -40

Repository: utopia-php/database

Length of output: 1587

🏁 Script executed:

#!/bin/bash
# Check imports and understand what Collection object is
rg '^use.*Collection|new Collection' src/Database/Database.php | head -20

Repository: utopia-php/database

Length of output: 45

🏁 Script executed:

#!/bin/bash
# Look at the actual return type of getCollection to understand what object type it is
rg -B 5 'getCollection.*{' src/Database/Database.php | head -40

Repository: utopia-php/database

Length of output: 158

🏁 Script executed:

#!/bin/bash
# Check Document class and see if it has getTenant method and how it stores tenant
rg -B 5 -A 20 'class Document' src/Database/Document.php | head -60

Repository: utopia-php/database

Length of output: 768

🏁 Script executed:

#!/bin/bash
# Check Document getTenant implementation
rg -A 5 'public function getTenant' src/Database/Document.php

Repository: utopia-php/database

Length of output: 179

🏁 Script executed:

#!/bin/bash
# Check if Document stores tenant as its own property or delegates to something else
rg 'getTenant|setTenant|\$tenant' src/Database/Document.php | head -30

Repository: utopia-php/database

Length of output: 158

🏁 Script executed:

#!/bin/bash
# Check if there are any tests that specifically test tenant comparison edge cases
rg -i 'tenant.*01|tenant.*1e|tenant.*0' tests/ --type php

Repository: utopia-php/database

Length of output: 45

🏁 Script executed:

#!/bin/bash
# Do a final comprehensive check for all tenant comparisons in the entire codebase
rg 'getTenant.*!=|!=.*getTenant' src/ --type php

Repository: utopia-php/database

Length of output: 850

🏁 Script executed:

#!/bin/bash
# Check if there are any actual string tenant values used anywhere
rg "setTenant\(['\"]" tests/ --type php

Repository: utopia-php/database

Length of output: 45

🏁 Script executed:

#!/bin/bash
# Verify the fix would work by checking if the proposed isSameTenant method signature is compatible with all call sites
rg 'getTenant\(\)' src/Database/Database.php | wc -l

Repository: utopia-php/database

Length of output: 65

Avoid loose tenant equality in multi-tenant authorization guards.

Using != for tenant identity checks allows PHP loose comparison to treat distinct tenant IDs as equal. Specifically, numeric-like strings ('01' vs '1') and null-like values (null vs 0 or '') become indistinguishable, weakening tenant isolation and the tenant immutability constraint in shared-table scenarios.

Replace all six occurrences with a helper that normalizes int/string types while maintaining strict null checks:

Proposed fix 
+    /**
+     * Tenant equality that allows int/string representation matches
+     * without PHP loose-comparison edge cases.
+     */
+    private function isSameTenant(int|string|null $left, int|string|null $right): bool
+    {
+        if ($left === null || $right === null) {
+            return $left === $right;
+        }
+
+        return (string) $left === (string) $right;
+    }

Applies to: lines 1859, 1895, 1950, 1976, 2010, and 7204.

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

In `@src/Database/Database.php` at line 1859, The tenant comparison uses PHP loose
inequality (!=) which can mis-evaluate distinct tenant IDs; replace all
occurrences of the pattern comparing $collection->getTenant() and
$this->adapter->getTenant() with a dedicated helper (e.g.,
tenantIdsDiffer($a,$b) or normalizeAndCompareTenantIds($a,$b)) that normalizes
numeric-like strings to integers, leaves non-numeric strings as-is, and performs
strict comparison while preserving null (i.e., null is not coerced to 0 or '').
Implement the helper as a private static method in Database (or a nearby
utility) and update the six spots where the code currently does "&&
$collection->getTenant() != $this->adapter->getTenant()" to call
tenantIdsDiffer($collection->getTenant(), $this->adapter->getTenant()) instead.
Ensure the helper returns true when tenants differ and false when they are
identical after normalization.

) {
throw new NotFoundException('Collection not found');
}
Expand Down Expand Up @@ -1892,7 +1892,7 @@ public function getCollection(string $id): Document
$id !== self::METADATA
&& $this->adapter->getSharedTables()
&& $collection->getTenant() !== null
&& $collection->getTenant() !== $this->adapter->getTenant()
&& $collection->getTenant() != $this->adapter->getTenant()
) {
return new Document();
}
Expand Down Expand Up @@ -1947,7 +1947,7 @@ public function getSizeOfCollection(string $collection): int
throw new NotFoundException('Collection not found');
}

if ($this->adapter->getSharedTables() && $collection->getTenant() !== $this->adapter->getTenant()) {
if ($this->adapter->getSharedTables() && $collection->getTenant() != $this->adapter->getTenant()) {
throw new NotFoundException('Collection not found');
}

Expand All @@ -1973,7 +1973,7 @@ public function getSizeOfCollectionOnDisk(string $collection): int
throw new NotFoundException('Collection not found');
}

if ($this->adapter->getSharedTables() && $collection->getTenant() !== $this->adapter->getTenant()) {
if ($this->adapter->getSharedTables() && $collection->getTenant() != $this->adapter->getTenant()) {
throw new NotFoundException('Collection not found');
}

Expand Down Expand Up @@ -2007,7 +2007,7 @@ public function deleteCollection(string $id): bool
throw new NotFoundException('Collection not found');
}

if ($this->adapter->getSharedTables() && $collection->getTenant() !== $this->adapter->getTenant()) {
if ($this->adapter->getSharedTables() && $collection->getTenant() != $this->adapter->getTenant()) {
throw new NotFoundException('Collection not found');
}

Expand Down Expand Up @@ -7201,7 +7201,7 @@ public function upsertDocumentsWithIncrease(
if ($document->getTenant() === null) {
throw new DatabaseException('Missing tenant. Tenant must be set when tenant per document is enabled.');
}
if (!$old->isEmpty() && $old->getTenant() !== $document->getTenant()) {
if (!$old->isEmpty() && $old->getTenant() != $document->getTenant()) {
throw new DatabaseException('Tenant cannot be changed.');
}
} else {
Expand Down
3 changes: 3 additions & 0 deletions src/Database/Validator/Structure.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,9 @@ protected function checkForInvalidAttributeValues(array $structure, array $keys)

switch ($type) {
case Database::VAR_ID:
if ($attribute['$id'] === '$tenant') {
break;
}
$validators[] = new Sequence($this->idAttributeType, $attribute['$id'] === '$sequence');
break;

Expand Down
Loading