Skip to content

Commit 59c363c

Browse files
github-actions[bot]claude-bot
authored andcommitted
fix(sqlite): close cross-tenant leaks in perms, FTS5, and cache
Four data-correctness/security bugs surfaced by reviewing the parity branch as a whole. - getSQLPermissionsCondition: the SQLite override dropped both the Database::PERMISSIONS whitelist and the getTenantQuery filter. Under shared tables, two tenants holding the same _uid would see each other's perms — cross-tenant authorization bypass. The parent in SQL.php works for SQLite via the SQLite getSQLTable override, so the override is just removed. - FTS5 triggers and backfill: under shared tables, AFTER INSERT/UPDATE/ DELETE triggers fire on the parent table for every tenant, so tenant A's vtable physically stored tenant B's tokenized content. Add WHEN NEW/OLD._tenant IS <tenant> to each trigger and WHERE _tenant IS <tenant> to the initial backfill, baked in as a SQL literal via getTenantSqlLiteral(). - ftsTableCache: keyed by collection only, so under Swoole pooling the same adapter instance serving tenant A then tenant B returned A's attribute->vtable map for B. Override setTenant/setNamespace/ setSharedTables to invalidate the cache when scope changes. - deleteAttribute/renameIndex: \json_decode(<array>) on the indexes attribute returned null (Document already returns the decoded array), so the foreach silently iterated nothing — index updates on attribute delete/rename were no-ops.
1 parent ea37b56 commit 59c363c

1 file changed

Lines changed: 61 additions & 55 deletions

File tree

src/Database/Adapter/SQLite.php

Lines changed: 61 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,31 @@ public function getEmulateMySQL(): bool
104104
return $this->emulateMySQL;
105105
}
106106

107+
public function setTenant(int|string|null $tenant): bool
108+
{
109+
if ($this->tenant !== $tenant) {
110+
$this->ftsTableCache = [];
111+
}
112+
113+
return parent::setTenant($tenant);
114+
}
115+
116+
public function setNamespace(string $namespace): static
117+
{
118+
$this->ftsTableCache = [];
119+
120+
return parent::setNamespace($namespace);
121+
}
122+
123+
public function setSharedTables(bool $sharedTables): bool
124+
{
125+
if ($this->sharedTables !== $sharedTables) {
126+
$this->ftsTableCache = [];
127+
}
128+
129+
return parent::setSharedTables($sharedTables);
130+
}
131+
107132
/**
108133
* Register a preg_match-backed REGEXP UDF so the inherited REGEXP
109134
* path resolves. Best-effort — non-SQLite PDOs simply skip it.
@@ -379,17 +404,10 @@ public function createCollection(string $name, array $attributes = [], array $in
379404
->prepare($permissions)
380405
->execute();
381406

382-
// getSQLIndex automatically prepends `_tenant` to every index
383-
// when sharedTables is on, so the resulting UNIQUE here is
384-
// already composite on (_tenant, _uid) under shared tables.
385407
$this->createIndex($id, '_index1', Database::INDEX_UNIQUE, ['_uid'], [], []);
386408
$this->createIndex($id, '_created_at', Database::INDEX_KEY, [ '_createdAt'], [], []);
387409
$this->createIndex($id, '_updated_at', Database::INDEX_KEY, [ '_updatedAt'], [], []);
388410

389-
// getSQLIndex prepends `_tenant` automatically under shared
390-
// tables, so the resulting UNIQUE on the perms table is
391-
// (_tenant, _document, _type, _permission) and two tenants
392-
// can hold the same role on the same document.
393411
$this->createIndex("{$id}_perms", '_index_1', Database::INDEX_UNIQUE, ['_document', '_type', '_permission'], [], []);
394412
$this->createIndex("{$id}_perms", '_index_2', Database::INDEX_KEY, ['_permission', '_type'], [], []);
395413

@@ -407,10 +425,6 @@ public function createCollection(string $name, array $attributes = [], array $in
407425

408426
$this->createIndex($id, $indexId, $indexType, $indexAttributes, $indexLengths, $indexOrders, [], [], $indexTtl);
409427
}
410-
411-
$this->createIndex("{$id}_perms", '_index_1', Database::INDEX_UNIQUE, ['_document', '_type', '_permission'], [], []);
412-
$this->createIndex("{$id}_perms", '_index_2', Database::INDEX_KEY, ['_permission', '_type'], [], []);
413-
414428
} catch (PDOException $e) {
415429
throw $this->processException($e);
416430
}
@@ -558,7 +572,7 @@ public function updateAttribute(string $collection, string $id, string $type, in
558572
$stmt = $this->getPDO()->prepare($sql);
559573
$stmt->bindValue(':max', $size, PDO::PARAM_INT);
560574
if ($this->sharedTables) {
561-
$stmt->bindValue(':_tenant', $this->tenant, PDO::PARAM_INT);
575+
$stmt->bindValue(':_tenant', $this->tenant, \is_int($this->tenant) ? PDO::PARAM_INT : PDO::PARAM_STR);
562576
}
563577
$stmt->execute();
564578

@@ -594,7 +608,7 @@ public function deleteAttribute(string $collection, string $id, bool $array = fa
594608
throw new NotFoundException('Collection not found');
595609
}
596610

597-
$indexes = \json_decode($collection->getAttribute('indexes', []), true);
611+
$indexes = $collection->getAttribute('indexes', []);
598612

599613
foreach ($indexes as $index) {
600614
$attributes = $index['attributes'];
@@ -644,7 +658,7 @@ public function renameIndex(string $collection, string $old, string $new): bool
644658

645659
$old = $this->filter($old);
646660
$new = $this->filter($new);
647-
$indexes = \json_decode($collection->getAttribute('indexes', []), true);
661+
$indexes = $collection->getAttribute('indexes', []);
648662
$index = null;
649663

650664
foreach ($indexes as $node) {
@@ -746,34 +760,40 @@ protected function createFulltextIndex(string $collection, string $id, array $at
746760
}
747761

748762
$columns = \array_map(fn (string $attr) => $this->filter($attr), $attributes);
749-
// Bare identifiers in fts5(...) — backticks aren't part of FTS5's
750-
// config grammar.
751763
$ftsColumnList = \implode(', ', $columns);
752764
$columnList = \implode(', ', \array_map(fn (string $c) => "`{$c}`", $columns));
753765
$newColumnList = \implode(', ', \array_map(fn (string $c) => "NEW.`{$c}`", $columns));
754766
$oldColumnList = \implode(', ', \array_map(fn (string $c) => "OLD.`{$c}`", $columns));
755767

756-
// Atomic setup so mid-backfill failure can't leave triggers
757-
// wired to a half-populated index.
768+
// Under shared tables every tenant has a distinct FTS vtable but the
769+
// parent table is shared, so triggers must filter by `_tenant`
770+
// literal — otherwise tenant A's vtable accumulates tenant B's
771+
// tokenized content. The same applies to the initial backfill.
772+
$tenantLiteral = $this->sharedTables ? $this->getTenantSqlLiteral() : null;
773+
$insertWhen = $tenantLiteral !== null ? " WHEN NEW.`_tenant` IS {$tenantLiteral}" : '';
774+
$deleteWhen = $tenantLiteral !== null ? " WHEN OLD.`_tenant` IS {$tenantLiteral}" : '';
775+
$updateWhen = $tenantLiteral !== null
776+
? " WHEN OLD.`_tenant` IS {$tenantLiteral} OR NEW.`_tenant` IS {$tenantLiteral}"
777+
: '';
778+
$backfillWhere = $tenantLiteral !== null ? " WHERE `_tenant` IS {$tenantLiteral}" : '';
779+
758780
$this->startTransaction();
759781
try {
760-
// Double quotes for content= — backticks aren't FTS5 grammar
761-
// and some builds record them as part of the literal value.
762782
$createSql = "CREATE VIRTUAL TABLE `{$ftsTable}` USING fts5({$ftsColumnList}, content=\"{$parentTable}\", content_rowid=\"_id\")";
763783
$createSql = $this->trigger(Database::EVENT_INDEX_CREATE, $createSql);
764784
$this->getPDO()->prepare($createSql)->execute();
765785

766786
$insertSuffix = self::FTS_TRIGGER_INSERT;
767787
$insertTrigger = "
768-
CREATE TRIGGER `{$ftsTable}_{$insertSuffix}` AFTER INSERT ON `{$parentTable}` BEGIN
788+
CREATE TRIGGER `{$ftsTable}_{$insertSuffix}` AFTER INSERT ON `{$parentTable}`{$insertWhen} BEGIN
769789
INSERT INTO `{$ftsTable}` (rowid, {$columnList}) VALUES (NEW.`_id`, {$newColumnList});
770790
END
771791
";
772792
$this->getPDO()->prepare($insertTrigger)->execute();
773793

774794
$deleteSuffix = self::FTS_TRIGGER_DELETE;
775795
$deleteTrigger = "
776-
CREATE TRIGGER `{$ftsTable}_{$deleteSuffix}` AFTER DELETE ON `{$parentTable}` BEGIN
796+
CREATE TRIGGER `{$ftsTable}_{$deleteSuffix}` AFTER DELETE ON `{$parentTable}`{$deleteWhen} BEGIN
777797
INSERT INTO `{$ftsTable}` (`{$ftsTable}`, rowid, {$columnList}) VALUES ('delete', OLD.`_id`, {$oldColumnList});
778798
END
779799
";
@@ -782,14 +802,14 @@ protected function createFulltextIndex(string $collection, string $id, array $at
782802
$updateSuffix = self::FTS_TRIGGER_UPDATE;
783803
// OF <cols>: skip re-tokenise when only timestamps/permissions change.
784804
$updateTrigger = "
785-
CREATE TRIGGER `{$ftsTable}_{$updateSuffix}` AFTER UPDATE OF {$columnList} ON `{$parentTable}` BEGIN
805+
CREATE TRIGGER `{$ftsTable}_{$updateSuffix}` AFTER UPDATE OF {$columnList} ON `{$parentTable}`{$updateWhen} BEGIN
786806
INSERT INTO `{$ftsTable}` (`{$ftsTable}`, rowid, {$columnList}) VALUES ('delete', OLD.`_id`, {$oldColumnList});
787807
INSERT INTO `{$ftsTable}` (rowid, {$columnList}) VALUES (NEW.`_id`, {$newColumnList});
788808
END
789809
";
790810
$this->getPDO()->prepare($updateTrigger)->execute();
791811

792-
$backfill = "INSERT INTO `{$ftsTable}` (rowid, {$columnList}) SELECT `_id`, {$columnList} FROM `{$parentTable}`";
812+
$backfill = "INSERT INTO `{$ftsTable}` (rowid, {$columnList}) SELECT `_id`, {$columnList} FROM `{$parentTable}`{$backfillWhere}";
793813
$this->getPDO()->prepare($backfill)->execute();
794814

795815
$this->commitTransaction();
@@ -847,6 +867,22 @@ private function getTenantSegment(): string
847867
return $this->filter((string) ($this->tenant ?? ''));
848868
}
849869

870+
/**
871+
* Tenant rendered as a SQL literal for embedding in trigger bodies and
872+
* other places where parameter binding isn't available.
873+
*/
874+
private function getTenantSqlLiteral(): string
875+
{
876+
if ($this->tenant === null) {
877+
return 'NULL';
878+
}
879+
if (\is_int($this->tenant)) {
880+
return (string) $this->tenant;
881+
}
882+
883+
return $this->getPDO()->quote((string) $this->tenant);
884+
}
885+
850886
/**
851887
* Delete Index
852888
*
@@ -1152,11 +1188,7 @@ public function createDocument(Document $collection, Document $document): Docume
11521188
try {
11531189
$stmt->execute();
11541190

1155-
$statment = $this->getPDO()->prepare("SELECT last_insert_rowid() AS id");
1156-
$statment->execute();
1157-
$last = $statment->fetch();
1158-
1159-
$document['$sequence'] = $last['id'];
1191+
$document['$sequence'] = (int) $this->getPDO()->lastInsertId();
11601192

11611193
if (isset($stmtPermissions)) {
11621194
$stmtPermissions->execute();
@@ -1696,26 +1728,6 @@ protected function getSQLIndex(string $collection, string $id, string $type, arr
16961728
return "CREATE {$type} {$key} ON `{$this->getNamespace()}_{$collection}` ({$attributes})";
16971729
}
16981730

1699-
/**
1700-
* Get SQL condition for permissions
1701-
*
1702-
* @param string $collection
1703-
* @param array<string> $roles
1704-
* @return string
1705-
* @throws Exception
1706-
*/
1707-
protected function getSQLPermissionsCondition(string $collection, array $roles, string $alias, string $type = Database::PERMISSION_READ): string
1708-
{
1709-
$roles = array_map(fn (string $role) => $this->getPDO()->quote($role), $roles);
1710-
1711-
return "{$this->quote($alias)}.{$this->quote('_uid')} IN (
1712-
SELECT distinct(_document)
1713-
FROM `{$this->getNamespace()}_{$collection}_perms`
1714-
WHERE _permission IN (" . implode(', ', $roles) . ")
1715-
AND _type = '{$type}'
1716-
)";
1717-
}
1718-
17191731
/**
17201732
* Get SQL table
17211733
*
@@ -2605,9 +2617,6 @@ public function createRelationship(
26052617
return true;
26062618
}
26072619

2608-
/**
2609-
* Same multi-statement split rationale as createRelationship.
2610-
*/
26112620
public function updateRelationship(
26122621
string $collection,
26132622
string $relatedCollection,
@@ -2692,9 +2701,6 @@ public function updateRelationship(
26922701
return true;
26932702
}
26942703

2695-
/**
2696-
* Same multi-statement split rationale as createRelationship.
2697-
*/
26982704
public function deleteRelationship(
26992705
string $collection,
27002706
string $relatedCollection,

0 commit comments

Comments
 (0)