From 67188349221a9c61bdd457198a2a03ada4f6daa1 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 28 Aug 2026 12:16:19 +1200 Subject: [PATCH 1/2] fix(adapter): write permission rows with the document's tenant on update createDocument binds the document's own tenant for the rows it writes to the _perms side table; updateDocument bound the adapter's. The two only diverge for a row whose tenant is not the selected one, which on a shared pool is the null-tenant _metadata row of a collection created once for every tenant: getTenantQuery() carves out `OR _tenant IS NULL` for Database::METADATA, so such a row is readable and updatable with any tenant selected. Updating one under a tenant therefore left the document row on tenant NULL and its permission rows on that tenant. Permission filtering reads those rows, so the definition became invisible to every other tenant on the pool. SQLite's own updateDocument went further and rewrote the document row's _tenant column to the selected tenant, moving the row off the shared scope entirely. Postgres is fixed for consistency with its createDocument; its permission filtering reads the denormalised _permissions column rather than the side table, so the regression test cannot observe the difference there. Co-Authored-By: Claude Opus 5 --- src/Database/Adapter/MariaDB.php | 8 +++- src/Database/Adapter/Postgres.php | 8 +++- src/Database/Adapter/SQLite.php | 9 ++-- tests/e2e/Adapter/Scopes/PermissionTests.php | 47 ++++++++++++++++++++ 4 files changed, 65 insertions(+), 7 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index f20f151f4..95e60f8e2 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1001,7 +1001,11 @@ public function updateDocument(Document $collection, string $id, Document $docum $stmtRemovePermissions = $this->getPDO()->prepare($sql); $stmtRemovePermissions->bindValue(':_uid', $id); if ($this->sharedTables) { - $stmtRemovePermissions->bindValue(':_tenant', $this->tenant); + // The document's own tenant, not the adapter's: a row whose + // tenant differs from the selected one (a shared collection's + // null-tenant _metadata row) would otherwise keep its old + // permissions and gain a second, wrongly-tenanted copy. + $stmtRemovePermissions->bindValue(':_tenant', $document->getTenant()); } $values = []; @@ -1026,7 +1030,7 @@ public function updateDocument(Document $collection, string $id, Document $docum $stmtAddPermissions = $this->getPDO()->prepare($sql); $stmtAddPermissions->bindValue(":_uid", $newUid); if ($this->sharedTables) { - $stmtAddPermissions->bindValue(":_tenant", $this->tenant); + $stmtAddPermissions->bindValue(":_tenant", $document->getTenant()); } foreach ($binds as $key => $permission) { diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index 0271db502..ba679cd63 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -1127,7 +1127,11 @@ public function updateDocument(Document $collection, string $id, Document $docum $stmtRemovePermissions = $this->getPDO()->prepare($sql); $stmtRemovePermissions->bindValue(':_uid', $id); if ($this->sharedTables) { - $stmtRemovePermissions->bindValue(':_tenant', $this->tenant); + // The document's own tenant, not the adapter's: a row whose + // tenant differs from the selected one (a shared collection's + // null-tenant _metadata row) would otherwise keep its old + // permissions and gain a second, wrongly-tenanted copy. + $stmtRemovePermissions->bindValue(':_tenant', $document->getTenant()); } $values = []; @@ -1152,7 +1156,7 @@ public function updateDocument(Document $collection, string $id, Document $docum $stmtAddPermissions = $this->getPDO()->prepare($sql); $stmtAddPermissions->bindValue(":_uid", $newUid); if ($this->sharedTables) { - $stmtAddPermissions->bindValue(':_tenant', $this->tenant); + $stmtAddPermissions->bindValue(':_tenant', $document->getTenant()); } foreach ($binds as $key => $permission) { diff --git a/src/Database/Adapter/SQLite.php b/src/Database/Adapter/SQLite.php index 92cf01569..25f86205c 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -1269,7 +1269,10 @@ public function updateDocument(Document $collection, string $id, Document $docum $attributes['_uid'] = $document->getId(); if ($this->sharedTables) { - $attributes['_tenant'] = $this->tenant; + // The document's own tenant, not the adapter's: writing the + // selected tenant here moves a row (a shared collection's + // null-tenant _metadata row) onto whoever happened to be selected. + $attributes['_tenant'] = $document->getTenant(); } $name = $this->filter($collection); @@ -1289,7 +1292,7 @@ public function updateDocument(Document $collection, string $id, Document $docum $stmtRemovePermissions = $this->getPDO()->prepare($sql); $stmtRemovePermissions->bindValue(':_uid', $id); if ($this->sharedTables) { - $stmtRemovePermissions->bindValue(':_tenant', $this->tenant); + $stmtRemovePermissions->bindValue(':_tenant', $document->getTenant()); } $values = []; @@ -1314,7 +1317,7 @@ public function updateDocument(Document $collection, string $id, Document $docum $stmtAddPermissions = $this->getPDO()->prepare($sql); $stmtAddPermissions->bindValue(":_uid", $newUid); if ($this->sharedTables) { - $stmtAddPermissions->bindValue(":_tenant", $this->tenant); + $stmtAddPermissions->bindValue(":_tenant", $document->getTenant()); } foreach ($binds as $key => $permission) { diff --git a/tests/e2e/Adapter/Scopes/PermissionTests.php b/tests/e2e/Adapter/Scopes/PermissionTests.php index a97f9691c..97e55633f 100644 --- a/tests/e2e/Adapter/Scopes/PermissionTests.php +++ b/tests/e2e/Adapter/Scopes/PermissionTests.php @@ -3,6 +3,7 @@ namespace Tests\E2E\Adapter\Scopes; use Exception; +use Utopia\Database\Adapter\SQL; use Utopia\Database\Database; use Utopia\Database\Document; use Utopia\Database\Exception as DatabaseException; @@ -10,9 +11,55 @@ use Utopia\Database\Helpers\ID; use Utopia\Database\Helpers\Permission; use Utopia\Database\Helpers\Role; +use Utopia\Database\Query; trait PermissionTests { + public function testUpdatingASharedDefinitionKeepsItsPermissionRowsTenantless(): void + { + /** @var Database $database */ + $database = $this->getDatabase(); + + // Only the SQL adapters keep permissions in a side table that carries + // its own tenant column; Mongo stores them on the document itself. + if (!$database->getSharedTables() || !$database->getAdapter() instanceof SQL) { + $this->expectNotToPerformAssertions(); + return; + } + + $tenant = $database->getTenant(); + $collection = 'sharedDefinitionPerms'; + + try { + // A shared pool's system collections are created once with no + // tenant, so every tenant on the pool reads the one definition. + $database->setTenant(null); + $database->createCollection($collection, [], [], [Permission::read(Role::any())], false); + + // A per-project pass rewrites that definition while it holds one + // project's tenant. The rows it writes belong to the document, not + // to whoever happened to be selected. + $database->setTenant(989); + $database->updateDocument(Database::METADATA, $collection, new Document([ + '$id' => $collection, + '$permissions' => [Permission::read(Role::any()), Permission::update(Role::any())], + ])); + + // Permission filtering reads the permission rows, so tenanting them + // to 989 hides the shared definition from every other tenant. + $database->setTenant(990); + $found = $database->find(Database::METADATA, [Query::equal('$id', [$collection])]); + + $this->assertCount( + 1, + $found, + 'A shared definition updated under one tenant must stay visible to the rest of the pool.', + ); + } finally { + $database->setTenant($tenant); + } + } + public function testUnsetPermissions(): void { /** @var Database $database */ From db47d8b6ebfbcf21f8843e99c53fe384694f17a5 Mon Sep 17 00:00:00 2001 From: Jake Barnby Date: Fri, 28 Aug 2026 12:23:19 +1200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Jake Barnby --- src/Database/Adapter/MariaDB.php | 4 ---- src/Database/Adapter/Postgres.php | 4 ---- src/Database/Adapter/SQLite.php | 3 --- 3 files changed, 11 deletions(-) diff --git a/src/Database/Adapter/MariaDB.php b/src/Database/Adapter/MariaDB.php index 95e60f8e2..6d2aac8ef 100644 --- a/src/Database/Adapter/MariaDB.php +++ b/src/Database/Adapter/MariaDB.php @@ -1001,10 +1001,6 @@ public function updateDocument(Document $collection, string $id, Document $docum $stmtRemovePermissions = $this->getPDO()->prepare($sql); $stmtRemovePermissions->bindValue(':_uid', $id); if ($this->sharedTables) { - // The document's own tenant, not the adapter's: a row whose - // tenant differs from the selected one (a shared collection's - // null-tenant _metadata row) would otherwise keep its old - // permissions and gain a second, wrongly-tenanted copy. $stmtRemovePermissions->bindValue(':_tenant', $document->getTenant()); } diff --git a/src/Database/Adapter/Postgres.php b/src/Database/Adapter/Postgres.php index ba679cd63..124112326 100644 --- a/src/Database/Adapter/Postgres.php +++ b/src/Database/Adapter/Postgres.php @@ -1127,10 +1127,6 @@ public function updateDocument(Document $collection, string $id, Document $docum $stmtRemovePermissions = $this->getPDO()->prepare($sql); $stmtRemovePermissions->bindValue(':_uid', $id); if ($this->sharedTables) { - // The document's own tenant, not the adapter's: a row whose - // tenant differs from the selected one (a shared collection's - // null-tenant _metadata row) would otherwise keep its old - // permissions and gain a second, wrongly-tenanted copy. $stmtRemovePermissions->bindValue(':_tenant', $document->getTenant()); } diff --git a/src/Database/Adapter/SQLite.php b/src/Database/Adapter/SQLite.php index 25f86205c..3880aec16 100644 --- a/src/Database/Adapter/SQLite.php +++ b/src/Database/Adapter/SQLite.php @@ -1269,9 +1269,6 @@ public function updateDocument(Document $collection, string $id, Document $docum $attributes['_uid'] = $document->getId(); if ($this->sharedTables) { - // The document's own tenant, not the adapter's: writing the - // selected tenant here moves a row (a shared collection's - // null-tenant _metadata row) onto whoever happened to be selected. $attributes['_tenant'] = $document->getTenant(); }