From 06d4292fa7631e7668a3f5b1ee5451026958e4ae Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 16 Mar 2026 01:16:30 +0000 Subject: [PATCH 1/6] feat: support int, string, or null types for tenant Update tenant property and methods across ClickHouse adapter and Log model to accept int|string|null instead of ?int. Change ClickHouse column type from Nullable(UInt64) to Nullable(String) to store both numeric and string tenant identifiers. https://claude.ai/code/session_01X8MFsu464fPHGaF7uPeWAZ --- src/Audit/Adapter/ClickHouse.php | 19 ++++++++++--------- src/Audit/Log.php | 8 ++++---- tests/Audit/Adapter/ClickHouseTest.php | 6 +++++- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/Audit/Adapter/ClickHouse.php b/src/Audit/Adapter/ClickHouse.php index 0d598d5..7793d83 100644 --- a/src/Audit/Adapter/ClickHouse.php +++ b/src/Audit/Adapter/ClickHouse.php @@ -42,7 +42,7 @@ class ClickHouse extends SQL protected string $namespace = ''; - protected ?int $tenant = null; + protected int|string|null $tenant = null; protected bool $sharedTables = false; @@ -208,10 +208,10 @@ public function getNamespace(): string * Set the tenant ID for multi-tenant support. * Tenant is used to isolate audit logs by tenant. * - * @param int|null $tenant + * @param int|string|null $tenant * @return self */ - public function setTenant(?int $tenant): self + public function setTenant(int|string|null $tenant): self { $this->tenant = $tenant; return $this; @@ -220,9 +220,9 @@ public function setTenant(?int $tenant): self /** * Get the tenant ID. * - * @return int|null + * @return int|string|null */ - public function getTenant(): ?int + public function getTenant(): int|string|null { return $this->tenant; } @@ -631,7 +631,7 @@ public function setup(): void // Add tenant column only if tables are shared across tenants if ($this->sharedTables) { - $columns[] = 'tenant Nullable(UInt64)'; // Supports 11-digit MySQL auto-increment IDs + $columns[] = 'tenant Nullable(String)'; } // Build indexes from base adapter schema @@ -1197,13 +1197,13 @@ private function parseJsonResults(string $result): array $document[$columnName] = $value ?? []; } } elseif ($columnName === 'tenant') { - // Parse tenant as integer or null + // Parse tenant as int, string, or null if ($value === null || $value === '') { $document[$columnName] = null; } elseif (is_numeric($value)) { $document[$columnName] = (int) $value; } else { - $document[$columnName] = null; + $document[$columnName] = (string) $value; } } elseif ($columnName === 'time') { // Convert ClickHouse timestamp format back to ISO 8601 @@ -1279,7 +1279,8 @@ private function getTenantFilter(): string } $escapedTenant = $this->escapeIdentifier('tenant'); - return " AND {$escapedTenant} = {$this->tenant}"; + $tenantValue = "'" . addslashes((string) $this->tenant) . "'"; + return " AND {$escapedTenant} = {$tenantValue}"; } /** diff --git a/src/Audit/Log.php b/src/Audit/Log.php index ffbf197..6708008 100644 --- a/src/Audit/Log.php +++ b/src/Audit/Log.php @@ -126,9 +126,9 @@ public function getData(): array /** * Get the tenant ID (for multi-tenant setups). * - * @return int|null + * @return int|string|null */ - public function getTenant(): ?int + public function getTenant(): int|string|null { $tenant = $this->getAttribute('tenant'); @@ -140,8 +140,8 @@ public function getTenant(): ?int return $tenant; } - if (is_numeric($tenant)) { - return (int) $tenant; + if (is_string($tenant)) { + return $tenant; } return null; diff --git a/tests/Audit/Adapter/ClickHouseTest.php b/tests/Audit/Adapter/ClickHouseTest.php index 1374c71..f252833 100644 --- a/tests/Audit/Adapter/ClickHouseTest.php +++ b/tests/Audit/Adapter/ClickHouseTest.php @@ -299,11 +299,15 @@ public function testSharedTablesConfiguration(): void $this->assertInstanceOf(ClickHouse::class, $result); $this->assertTrue($adapter->isSharedTables()); - // Test setting tenant + // Test setting tenant to int $result2 = $adapter->setTenant(12345); $this->assertInstanceOf(ClickHouse::class, $result2); $this->assertEquals(12345, $adapter->getTenant()); + // Test setting tenant to string + $adapter->setTenant('tenant_abc'); + $this->assertSame('tenant_abc', $adapter->getTenant()); + // Test setting tenant to null $adapter->setTenant(null); $this->assertNull($adapter->getTenant()); From 3f2ab03615a4da3f2fc57b0cb04708b196b5fd9d Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 16 Mar 2026 01:28:23 +0000 Subject: [PATCH 2/6] fix: use parameterized queries for tenant filter to prevent SQL injection Replace addslashes-based string interpolation with ClickHouse native query parameters ({_tenant:String}) for tenant filtering. All call sites now merge tenant params via getTenantParams(). https://claude.ai/code/session_01X8MFsu464fPHGaF7uPeWAZ --- src/Audit/Adapter/ClickHouse.php | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/Audit/Adapter/ClickHouse.php b/src/Audit/Adapter/ClickHouse.php index 7793d83..aa754de 100644 --- a/src/Audit/Adapter/ClickHouse.php +++ b/src/Audit/Adapter/ClickHouse.php @@ -799,7 +799,7 @@ public function getById(string $id): ?Log FORMAT JSON "; - $result = $this->query($sql, ['id' => $id]); + $result = $this->query($sql, array_merge(['id' => $id], $this->getTenantParams())); $logs = $this->parseJsonResults($result); return $logs[0] ?? null; @@ -850,7 +850,7 @@ public function find(array $queries = []): array FORMAT JSON "; - $result = $this->query($sql, $parsed['params']); + $result = $this->query($sql, array_merge($parsed['params'], $this->getTenantParams())); return $this->parseJsonResults($result); } @@ -890,7 +890,7 @@ public function count(array $queries = []): int FORMAT TabSeparated "; - $result = $this->query($sql, $params); + $result = $this->query($sql, array_merge($params, $this->getTenantParams())); $trimmed = trim($result); return $trimmed !== '' ? (int) $trimmed : 0; @@ -1279,8 +1279,21 @@ private function getTenantFilter(): string } $escapedTenant = $this->escapeIdentifier('tenant'); - $tenantValue = "'" . addslashes((string) $this->tenant) . "'"; - return " AND {$escapedTenant} = {$tenantValue}"; + return " AND {$escapedTenant} = {_tenant:String}"; + } + + /** + * Get query parameters for tenant filtering. + * + * @return array + */ + private function getTenantParams(): array + { + if (!$this->sharedTables || $this->tenant === null) { + return []; + } + + return ['_tenant' => (string) $this->tenant]; } /** @@ -1571,7 +1584,7 @@ public function cleanup(\DateTime $datetime): bool WHERE time < {datetime:String}{$tenantFilter} "; - $this->query($sql, ['datetime' => $datetimeString]); + $this->query($sql, array_merge(['datetime' => $datetimeString], $this->getTenantParams())); return true; } From 6a040ebb22fee43ed0af102b6ef8f193f8faeb9f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 16 Mar 2026 01:34:38 +0000 Subject: [PATCH 3/6] fix: add type check before casting mixed tenant value to string PHPStan does not allow casting mixed to string directly. Use is_scalar() guard before the cast. https://claude.ai/code/session_01X8MFsu464fPHGaF7uPeWAZ --- src/Audit/Adapter/ClickHouse.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Audit/Adapter/ClickHouse.php b/src/Audit/Adapter/ClickHouse.php index aa754de..1ebd7b7 100644 --- a/src/Audit/Adapter/ClickHouse.php +++ b/src/Audit/Adapter/ClickHouse.php @@ -1202,8 +1202,10 @@ private function parseJsonResults(string $result): array $document[$columnName] = null; } elseif (is_numeric($value)) { $document[$columnName] = (int) $value; - } else { + } elseif (is_scalar($value)) { $document[$columnName] = (string) $value; + } else { + $document[$columnName] = null; } } elseif ($columnName === 'time') { // Convert ClickHouse timestamp format back to ISO 8601 From a7f3251f63df526bfe3acda2c68008adcc2d224c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 16 Mar 2026 07:00:03 +0000 Subject: [PATCH 4/6] fix: return int for numeric tenant values in getTenant() Address review comment from abnegate: if tenant is numeric, cast to int before returning, maintaining backward compatibility with integer tenant IDs. https://claude.ai/code/session_01X8MFsu464fPHGaF7uPeWAZ --- src/Audit/Log.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Audit/Log.php b/src/Audit/Log.php index 6708008..f46ef69 100644 --- a/src/Audit/Log.php +++ b/src/Audit/Log.php @@ -140,6 +140,10 @@ public function getTenant(): int|string|null return $tenant; } + if (is_numeric($tenant)) { + return (int) $tenant; + } + if (is_string($tenant)) { return $tenant; } From c7abe55c010ddb47fc2538a7616c0a14f07abcd6 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 16 Mar 2026 07:02:23 +0000 Subject: [PATCH 5/6] revert: keep numeric string tenants as strings in getTenant() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Do not cast numeric strings to int — if tenant was set as a string, return it as a string. The is_int() check already handles actual integers. https://claude.ai/code/session_01X8MFsu464fPHGaF7uPeWAZ --- src/Audit/Log.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Audit/Log.php b/src/Audit/Log.php index f46ef69..6708008 100644 --- a/src/Audit/Log.php +++ b/src/Audit/Log.php @@ -140,10 +140,6 @@ public function getTenant(): int|string|null return $tenant; } - if (is_numeric($tenant)) { - return (int) $tenant; - } - if (is_string($tenant)) { return $tenant; } From 881546e7296c229689910820ff6166539ed5853c Mon Sep 17 00:00:00 2001 From: Damodar Lohani Date: Thu, 14 May 2026 03:26:20 +0000 Subject: [PATCH 6/6] fix: address greptile review on tenant scheme handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three follow-ups from the greptile review of the b71aafc merge: P1 — Default `idAttributeType` to VAR_INTEGER, not VAR_UUID7. Previously, any existing deployment that called `setup()` after this PR landed would silently trigger `reconcileTenantColumnType()` to `ALTER TABLE ... MODIFY COLUMN tenant Nullable(String)` on a previously-UInt64 column — an irreversible schema change firing without the caller knowing they needed to opt out. Defaulting to VAR_INTEGER preserves the previous schema; callers using string-typed tenant IDs must opt in explicitly via `setIdAttributeType(VAR_UUID7)`. P1 — Parser only casts numeric tenants to int in integer scheme. `parseJsonResults()` was unconditionally casting `is_numeric($value)` to int, which corrupts string-scheme tenants like "42" or "00123" — they came back from ClickHouse's `Nullable(String)` column as PHP strings, passed `is_numeric()`, and silently became `int(42)` / `int(123)`. Cast to int only when the configured scheme is VAR_INTEGER; otherwise keep the value verbatim as a string. P2 — Log::getTenant returns strings as-is. Same is_numeric-cast issue was layered in `getTenant()` as a belt-and- braces fallback, but with the parser fix above it's no longer needed and was actively harmful: a manually-constructed Log with tenant "42" in string mode would silently become int(42). Strings now return verbatim; the parser is the single source of truth for the int vs string decision. Adds a regression test that exercises a numeric-looking string tenant ("00123") in string mode and asserts `is_string` + strict string equality on the round-trip — would have caught the bug. --- src/Audit/Adapter/ClickHouse.php | 20 ++++++++--- src/Audit/Log.php | 9 +++-- tests/Audit/Adapter/ClickHouseTest.php | 50 ++++++++++++++++++++++---- 3 files changed, 64 insertions(+), 15 deletions(-) diff --git a/src/Audit/Adapter/ClickHouse.php b/src/Audit/Adapter/ClickHouse.php index 6510bcb..cbc5285 100644 --- a/src/Audit/Adapter/ClickHouse.php +++ b/src/Audit/Adapter/ClickHouse.php @@ -75,10 +75,16 @@ class ClickHouse extends SQL * * Determines the ClickHouse column type for the `tenant` column when * `setSharedTables(true)` is enabled: - * - `integer` → `Nullable(UInt64)` - * - anything else (default) → `Nullable(String)` + * - `integer` (default) → `Nullable(UInt64)` + * - anything else → `Nullable(String)` + * + * Defaults to `VAR_INTEGER` to preserve the previous schema on existing + * deployments: `reconcileTenantColumnType()` would otherwise trigger an + * irreversible `ALTER TABLE ... MODIFY COLUMN` from UInt64 to String on + * first `setup()` after upgrade. Callers using string IDs must opt in + * explicitly via {@see setIdAttributeType()}. */ - protected string $idAttributeType = Database::VAR_UUID7; + protected string $idAttributeType = Database::VAR_INTEGER; /** * @param string $host ClickHouse host @@ -1718,10 +1724,14 @@ private function parseJsonResults(string $result): array $document[$columnName] = $value ?? []; } } elseif ($columnName === 'tenant') { - // Parse tenant as int, string, or null + // Parse tenant according to the configured scheme: + // - integer scheme → cast numeric values to int + // - string scheme (uuid7 etc.) → keep verbatim so that + // numeric-looking IDs like "00123" or "42" round-trip + // without lossy coercion to int if ($value === null || $value === '') { $document[$columnName] = null; - } elseif (is_numeric($value)) { + } elseif ($this->idAttributeType === Database::VAR_INTEGER && is_numeric($value)) { $document[$columnName] = (int) $value; } elseif (is_scalar($value)) { $document[$columnName] = (string) $value; diff --git a/src/Audit/Log.php b/src/Audit/Log.php index 91c72eb..80da263 100644 --- a/src/Audit/Log.php +++ b/src/Audit/Log.php @@ -140,10 +140,13 @@ public function getTenant(): int|string|null return $tenant; } + // Strings are returned as-is. The ClickHouse parser converts tenant + // values to int when the integer scheme is configured, so a string + // here means the host application is using a string-typed tenant + // (uuid7, slug, etc.) — coercing numeric-looking strings like + // "00123" would silently corrupt them. if (is_string($tenant)) { - // Numeric tenant IDs (e.g. legacy UInt64 rows stored as "123") - // round-trip back to int. Non-numeric strings stay as-is. - return is_numeric($tenant) ? (int) $tenant : $tenant; + return $tenant; } return null; diff --git a/tests/Audit/Adapter/ClickHouseTest.php b/tests/Audit/Adapter/ClickHouseTest.php index 2c32fa2..c99d33f 100644 --- a/tests/Audit/Adapter/ClickHouseTest.php +++ b/tests/Audit/Adapter/ClickHouseTest.php @@ -778,15 +778,18 @@ public function testEqualRejectsEmptyValues(): void ]); } - public function testIdAttributeTypeDefaultsToString(): void + public function testIdAttributeTypeDefaultsToInteger(): void { + // Default is VAR_INTEGER to preserve the pre-existing schema on + // existing deployments — callers using string tenant IDs must opt + // in via setIdAttributeType(). $adapter = new ClickHouse( host: 'clickhouse', username: 'default', password: 'clickhouse' ); - $this->assertEquals(\Utopia\Database\Database::VAR_UUID7, $adapter->getIdAttributeType()); + $this->assertEquals(\Utopia\Database\Database::VAR_INTEGER, $adapter->getIdAttributeType()); } public function testIdAttributeTypeSetter(): void @@ -808,7 +811,8 @@ public function testTenantColumnTypeFollowsIdAttributeType(): void $chPort = (int) (getenv('CLICKHOUSE_PORT') ?: 8123); $required = $this->getRequiredAttributes(); - // Default scheme (uuid7) → tenant column is Nullable(String) + // String scheme (uuid7) → tenant column is Nullable(String). Must + // opt in explicitly — default is VAR_INTEGER for BC. $stringAdapter = new ClickHouse( host: $chHost, username: 'default', @@ -816,6 +820,7 @@ public function testTenantColumnTypeFollowsIdAttributeType(): void port: $chPort ); $stringAdapter->setNamespace('tenant_string_test'); + $stringAdapter->setIdAttributeType(\Utopia\Database\Database::VAR_UUID7); $stringAdapter->setSharedTables(true); $stringAdapter->setTenant('uuid-tenant-1'); $stringAdapter->setup(); @@ -824,9 +829,10 @@ public function testTenantColumnTypeFollowsIdAttributeType(): void $stringAudit->log('u1', 'create', 'doc/1', 'agent', '127.0.0.1', 'US', $required); $logs = $stringAudit->find([Query::equal('userId', 'u1')]); $this->assertGreaterThanOrEqual(1, count($logs)); - $this->assertEquals('uuid-tenant-1', $logs[0]->getTenant()); + $this->assertSame('uuid-tenant-1', $logs[0]->getTenant()); - // Integer scheme → tenant column is Nullable(UInt64), tenant is int + // Integer scheme (default) → tenant column is Nullable(UInt64), + // tenant is int. $intAdapter = new ClickHouse( host: $chHost, username: 'default', @@ -834,7 +840,6 @@ public function testTenantColumnTypeFollowsIdAttributeType(): void port: $chPort ); $intAdapter->setNamespace('tenant_int_test'); - $intAdapter->setIdAttributeType(\Utopia\Database\Database::VAR_INTEGER); $intAdapter->setSharedTables(true); $intAdapter->setTenant(42); $intAdapter->setup(); @@ -843,7 +848,38 @@ public function testTenantColumnTypeFollowsIdAttributeType(): void $intAudit->log('u1', 'create', 'doc/1', 'agent', '127.0.0.1', 'US', $required); $intLogs = $intAudit->find([Query::equal('userId', 'u1')]); $this->assertGreaterThanOrEqual(1, count($intLogs)); - // getTenant() returns int for numeric values regardless of column type $this->assertSame(42, $intLogs[0]->getTenant()); } + + public function testNumericStringTenantPreservedInStringMode(): void + { + // Regression test: in uuid7 mode, a tenant ID that looks numeric + // (e.g. "42" or "00123") must round-trip back as a string, not + // silently cast to int by the parser or by Log::getTenant(). + $chHost = getenv('CLICKHOUSE_HOST') ?: 'clickhouse'; + $chPort = (int) (getenv('CLICKHOUSE_PORT') ?: 8123); + + $adapter = new ClickHouse( + host: $chHost, + username: 'default', + password: 'clickhouse', + port: $chPort + ); + $adapter->setNamespace('tenant_numstr_test'); + $adapter->setIdAttributeType(\Utopia\Database\Database::VAR_UUID7); + $adapter->setSharedTables(true); + $adapter->setTenant('00123'); + $adapter->setup(); + + $audit = new Audit($adapter); + $audit->log('u1', 'create', 'doc/1', 'agent', '127.0.0.1', 'US', $this->getRequiredAttributes()); + + $logs = $audit->find([Query::equal('userId', 'u1')]); + $this->assertGreaterThanOrEqual(1, count($logs)); + + // Strict equality — must stay a string, not int(123) + $tenant = $logs[0]->getTenant(); + $this->assertIsString($tenant); + $this->assertSame('00123', $tenant); + } }