From 09d03ab704d690ef6ebb22fdf0669653a3b2b99c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 11:39:19 +0000 Subject: [PATCH] Write verify.json atomically, refuse corrupt files and fix database storage writeJson now encodes first, writes to a temporary file in the same directory and renames it into place, so a crash or full disk mid-write cannot leave a truncated list. Invalid UTF-8 in a record is substituted instead of making json_encode fail and writing an empty file. loadJsonFile used to turn any unreadable JSON into an empty list, which the next registration then persisted, silently wiping every verified account. It now throws; the request fails with a clear error instead. JSON_PATH may be absolute (Unix, drive-letter or UNC); it was always prefixed with the working directory. Database storage was unusable past the first record: rows read back carry an uid=0(root) gid=0(root) groups=0(root) column, and feeding them to the named-parameter insert failed with HY000/HY093. The table is now rewritten in a transaction with only the record columns, database mode no longer also writes the JSON file, and the connection is opened through an overridable connect() so the behaviour is covered by an in-memory SQLite test. Also fixes a by-reference return notice in the SS14 state's database read. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_016WTP2S4qKEFT8a6a6yFEL4 --- .../Endpoints/SS14VerifiedEndpoint.php | 3 +- .../Endpoints/VerifiedEndpoint.php | 6 +- src/VerifierServer/PersistentState.php | 88 +++++++++++-- src/VerifierServer/SS14PersistentState.php | 26 ++-- .../Traits/JsonStorageTrait.php | 28 ++++- tests/isolated/StorageIntegrityTest.php | 117 ++++++++++++++++++ 6 files changed, 231 insertions(+), 37 deletions(-) create mode 100644 tests/isolated/StorageIntegrityTest.php diff --git a/src/VerifierServer/Endpoints/SS14VerifiedEndpoint.php b/src/VerifierServer/Endpoints/SS14VerifiedEndpoint.php index d8a7a9b..6750102 100644 --- a/src/VerifierServer/Endpoints/SS14VerifiedEndpoint.php +++ b/src/VerifierServer/Endpoints/SS14VerifiedEndpoint.php @@ -156,8 +156,7 @@ public function add(string $discord, string $ss14): void 'ss14' => $ss14, 'create_time' => date('Y-m-d H:i:s') ]; - $this->state::writeJson($this->state->getJsonPath(), $list); - $this->state->setVerifyList($list); + $this->state->saveVerifyList($list); } public function remove(string $discord, string $ss14 = ''): ?array diff --git a/src/VerifierServer/Endpoints/VerifiedEndpoint.php b/src/VerifierServer/Endpoints/VerifiedEndpoint.php index 8b3b580..efc103e 100644 --- a/src/VerifierServer/Endpoints/VerifiedEndpoint.php +++ b/src/VerifierServer/Endpoints/VerifiedEndpoint.php @@ -275,8 +275,7 @@ public function add(string $discord, string $ckey): void 'ss13' => $ckey, 'create_time' => date('Y-m-d H:i:s') ]; - $this->state::writeJson($this->state->getJsonPath(), $list); - $this->state->setVerifyList($list); + $this->state->saveVerifyList($list); } public function remove(string $discord, string $ckey = ''): ?array @@ -298,8 +297,7 @@ public function removeIndex(int|string $existingIndex): array { $list = $this->state->getVerifyList(); $splice = array_splice($list, (int)$existingIndex, 1); - $this->state::writeJson($this->state->getJsonPath(), $list); - $this->state->setVerifyList($list); + $this->state->saveVerifyList($list); return $splice; } diff --git a/src/VerifierServer/PersistentState.php b/src/VerifierServer/PersistentState.php index 602e0a1..6ee86cf 100644 --- a/src/VerifierServer/PersistentState.php +++ b/src/VerifierServer/PersistentState.php @@ -67,13 +67,7 @@ public function __construct( */ protected function initializeDatabase(): void { - $env = self::loadEnvConfig(); - $this->pdo = new PDO( - $env['DB_DSN'], - $env['DB_USERNAME'], - $env['DB_PASSWORD'], - isset($env['DB_OPTIONS']) ? json_decode($env['DB_OPTIONS'], true) : null - ); + $this->pdo = $this->connect(); if (strpos($this->pdo->getAttribute(PDO::ATTR_DRIVER_NAME), 'mysql') !== false) { if ($this->pdo->exec("CREATE TABLE IF NOT EXISTS {$this->table_name} ( id INT AUTO_INCREMENT PRIMARY KEY, @@ -97,6 +91,44 @@ protected function initializeDatabase(): void } } + /** + * Opens the database connection from the environment configuration. + * Overridden by tests to inject an in-memory connection. + */ + protected function connect(): PDO + { + $env = self::loadEnvConfig(); + return new PDO( + $env['DB_DSN'], + $env['DB_USERNAME'], + $env['DB_PASSWORD'], + isset($env['DB_OPTIONS']) && $env['DB_OPTIONS'] !== '' ? json_decode($env['DB_OPTIONS'], true) : null + ); + } + + /** + * Columns persisted for each record. Rows read back from a database also carry `id`, + * which must not be fed back into the insert statement. + */ + protected function recordColumns(): array + { + return ['ss13', 'discord', 'create_time']; + } + + /** + * Persists a replacement list through the configured storage and updates the cache. + * Filesystem storage writes the JSON file; database storage rewrites the table. + */ + public function saveVerifyList(array $list): void + { + if ($this->storageType === 'filesystem') { + static::writeJson($this->getJsonPath(), $list); + $this->setVerifyList($list, false); + return; + } + $this->setVerifyList($list, true); + } + /** * Retrieves the verification list. * @@ -142,17 +174,40 @@ public function getVerifyList(bool $getLocalCache = false): array public function setVerifyList(array $list, bool $write = true): void { if ($write && $this->storageType !== 'filesystem') { + $this->rewriteTable($list); + } + $this->verifyList = $list; + } + + /** + * Replaces every row of the table inside one transaction with only the record columns. + * + * @throws PDOException If any statement fails; the transaction is rolled back. + */ + protected function rewriteTable(array $list): void + { + $columns = $this->recordColumns(); + $placeholders = implode(', ', array_map(fn($column) => ":$column", $columns)); + $this->pdo->beginTransaction(); + try { if ($this->pdo->exec("DELETE FROM {$this->table_name}") === false) { throw new PDOException("Failed to delete from {$this->table_name}: " . implode(", ", $this->pdo->errorInfo())); } - if (($stmt = $this->pdo->prepare("INSERT INTO {$this->table_name} (ss13, discord, create_time) VALUES (:ss13, :discord, :create_time)")) === false) { + if (($stmt = $this->pdo->prepare("INSERT INTO {$this->table_name} (" . implode(', ', $columns) . ") VALUES ($placeholders)")) === false) { throw new PDOException("Failed to prepare statement."); } - foreach ($list as $item) if (!$stmt->execute($item)) { - throw new PDOException("Failed to execute statement."); + foreach ($list as $item) { + $row = []; + foreach ($columns as $column) $row[$column] = isset($item[$column]) ? (string) $item[$column] : null; + if (!$stmt->execute($row)) { + throw new PDOException("Failed to execute statement."); + } } + $this->pdo->commit(); + } catch (\Throwable $e) { + if ($this->pdo->inTransaction()) $this->pdo->rollBack(); + throw $e; } - $this->verifyList = $list; } /** @@ -172,7 +227,16 @@ public function getToken(): string */ public function getJsonPath(): string { - return getcwd() . '/' . $this->json_path; + return self::isAbsolutePath($this->json_path) + ? $this->json_path + : getcwd() . '/' . $this->json_path; + } + + /** True for `/path`, `C:\path`, `C:/path` and `\\server\share` paths. */ + public static function isAbsolutePath(string $path): bool + { + return $path !== '' + && ($path[0] === '/' || $path[0] === '\\' || preg_match('~^[A-Za-z]:[\\\\/]~', $path) === 1); } /** diff --git a/src/VerifierServer/SS14PersistentState.php b/src/VerifierServer/SS14PersistentState.php index be8f4f8..a3bfaf1 100644 --- a/src/VerifierServer/SS14PersistentState.php +++ b/src/VerifierServer/SS14PersistentState.php @@ -51,13 +51,7 @@ public function __construct( */ protected function initializeDatabase(): void { - $env = self::loadEnvConfig(); - $this->pdo = new PDO( - $env['DB_DSN'], - $env['DB_USERNAME'], - $env['DB_PASSWORD'], - isset($env['DB_OPTIONS']) ? json_decode($env['DB_OPTIONS'], true) : null - ); + $this->pdo = $this->connect(); if (strpos($this->pdo->getAttribute(PDO::ATTR_DRIVER_NAME), 'mysql') !== false) { if ($this->pdo->exec("CREATE TABLE IF NOT EXISTS {$this->table_name} ( id INT AUTO_INCREMENT PRIMARY KEY, @@ -102,7 +96,8 @@ public function &getVerifyList(bool $getLocalCache = false): array if ($result === false) { throw new PDOException("Failed to fetch data: " . implode(", ", $this->pdo->errorInfo())); } - return $this->verifyList = $result; + $this->verifyList = $result; + return $this->verifyList; } /** @@ -122,16 +117,13 @@ public function &getVerifyList(bool $getLocalCache = false): array public function setVerifyList(array $list, bool $write = true): void { if ($write && $this->storageType !== 'filesystem') { - if ($this->pdo->exec("DELETE FROM {$this->table_name}") === false) { - throw new PDOException("Failed to delete from {$this->table_name}: " . implode(", ", $this->pdo->errorInfo())); - } - if (($stmt = $this->pdo->prepare("INSERT INTO {$this->table_name} (discord, ss14, create_time) VALUES (:discord, :ss14, :create_time)")) === false) { - throw new PDOException("Failed to prepare statement."); - } - foreach ($list as $item) if (!$stmt->execute($item)) { - throw new PDOException("Failed to execute statement."); - } + $this->rewriteTable($list); } $this->verifyList = $list; } + + protected function recordColumns(): array + { + return ['discord', 'ss14', 'create_time']; + } } \ No newline at end of file diff --git a/src/VerifierServer/Traits/JsonStorageTrait.php b/src/VerifierServer/Traits/JsonStorageTrait.php index 253a719..1dc36c1 100644 --- a/src/VerifierServer/Traits/JsonStorageTrait.php +++ b/src/VerifierServer/Traits/JsonStorageTrait.php @@ -26,17 +26,41 @@ public static function loadJsonFile(string $json_path): ?array if ($data === false) { throw new RuntimeException("Failed to read $json_path"); } - return json_decode($data, true) ?: []; + if (trim($data) === '') { + return []; + } + $decoded = json_decode($data, true); + if (!is_array($decoded)) { + // A damaged file must not silently become an empty list that the next write then persists. + throw new RuntimeException("Invalid JSON in $json_path: " . json_last_error_msg()); + } + return $decoded; } /** * Writes the given data to a file in JSON format. * + * The data is written to a temporary file next to the target and renamed into + * place, so a crash or full disk mid-write leaves the previous file intact. + * * @param string $file The path to the file where the JSON data will be written. * @param mixed $data The data to be encoded as JSON and written to the file. + * + * @throws RuntimeException If the data cannot be encoded or the file cannot be written. */ public static function writeJson(string $file, array $data): void { - file_put_contents($file, json_encode($data, JSON_PRETTY_PRINT)); + $json = json_encode($data, JSON_PRETTY_PRINT | JSON_INVALID_UTF8_SUBSTITUTE | JSON_THROW_ON_ERROR); + if (!is_dir($directory = dirname($file))) { + mkdir($directory, 0777, true); + } + $temporary = tempnam($directory, basename($file) . '.'); + if ($temporary === false) { + throw new RuntimeException("Failed to create a temporary file beside $file"); + } + if (file_put_contents($temporary, $json) !== strlen($json) || !rename($temporary, $file)) { + @unlink($temporary); + throw new RuntimeException("Failed to write $file"); + } } } \ No newline at end of file diff --git a/tests/isolated/StorageIntegrityTest.php b/tests/isolated/StorageIntegrityTest.php new file mode 100644 index 0000000..9f85169 --- /dev/null +++ b/tests/isolated/StorageIntegrityTest.php @@ -0,0 +1,117 @@ +directory = sys_get_temp_dir() . '/verifier-storage-' . bin2hex(random_bytes(4)); + mkdir($this->directory); + } + + protected function tearDown(): void + { + foreach (glob($this->directory . '/*') ?: [] as $file) unlink($file); + rmdir($this->directory); + } + + public function testCorruptFileIsRefusedInsteadOfBecomingAnEmptyList(): void + { + $file = $this->directory . '/verify.json'; + file_put_contents($file, '[{"discord":"123456789012345678","ss13":"ali'); + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage('Invalid JSON'); + PersistentState::loadJsonFile($file); + } + + public function testEmptyAndMissingFilesLoadAsEmptyLists(): void + { + $file = $this->directory . '/nested/verify.json'; + self::assertSame([], PersistentState::loadJsonFile($file)); + self::assertFileExists($file); + file_put_contents($file, "\n"); + self::assertSame([], PersistentState::loadJsonFile($file)); + unlink($file); + rmdir($this->directory . '/nested'); + } + + public function testWriteReplacesTheFileAtomicallyAndLeavesNoTemporaryFiles(): void + { + $file = $this->directory . '/verify.json'; + $records = [['discord' => '123456789012345678', 'ss13' => 'alice', 'create_time' => '2026-01-01 00:00:00']]; + PersistentState::writeJson($file, $records); + self::assertSame($records, json_decode(file_get_contents($file), true)); + PersistentState::writeJson($file, []); + self::assertSame([], json_decode(file_get_contents($file), true)); + self::assertSame(['verify.json'], array_map('basename', glob($this->directory . '/*'))); + } + + public function testInvalidUtf8IsSubstitutedRatherThanWritingAnEmptyFile(): void + { + $file = $this->directory . '/verify.json'; + PersistentState::writeJson($file, [['discord' => '123456789012345678', 'ss13' => "al\xffice"]]); + self::assertSame("al\u{FFFD}ice", json_decode(file_get_contents($file), true)[0]['ss13']); + } + + public function testAbsoluteJsonPathIsUsedAsIs(): void + { + $file = $this->directory . '/absolute.json'; + $state = new PersistentState('test-token', 'filesystem', $file); + self::assertSame($file, $state->getJsonPath()); + self::assertSame(getcwd() . '/json/verify.json', (new PersistentState('test-token'))->getJsonPath()); + self::assertTrue(PersistentState::isAbsolutePath('C:\\verifier\\verify.json')); + self::assertTrue(PersistentState::isAbsolutePath('D:/verifier/verify.json')); + self::assertTrue(PersistentState::isAbsolutePath('\\\\host\\share\\verify.json')); + self::assertFalse(PersistentState::isAbsolutePath('json/verify.json')); + $endpoint = new VerifiedEndpoint($state); + $endpoint->add('123456789012345678', 'alice'); + self::assertSame('alice', json_decode(file_get_contents($file), true)[0]['ss13']); + $reopened = new PersistentState('test-token', 'filesystem', $file); + self::assertSame('alice', $reopened->getVerifyList()[0]['ss13']); + } + + public function testDatabaseStorageSurvivesRepeatedInsertsAndDeletes(): void + { + $state = new MemoryDatabaseState('test-token', 'database', 'unused.json', 'verify_list'); + $endpoint = new VerifiedEndpoint($state); + $endpoint->add('123456789012345678', 'alice'); + $endpoint->add('234567890123456789', 'bob'); + $rows = $state->getVerifyList(); + self::assertCount(2, $rows); + self::assertSame(['alice', 'bob'], array_column($rows, 'ss13')); + self::assertArrayHasKey('id', $rows[0]); + self::assertSame(0, $endpoint->getIndex('123456789012345678')); + self::assertSame(['123456789012345678'], array_column($endpoint->remove('123456789012345678'), 'discord')); + self::assertSame(['bob'], array_column($state->getVerifyList(), 'ss13')); + $endpoint->add('345678901234567890', 'carol'); + self::assertSame(['bob', 'carol'], array_column($state->getVerifyList(), 'ss13')); + } + + public function testSs14DatabaseStorageUsesItsOwnColumns(): void + { + $state = new MemorySS14DatabaseState('test-token', 'database', 'unused.json', 'ss14_verify_list'); + $endpoint = new SS14VerifiedEndpoint($state); + $endpoint->add('123456789012345678', 'alice'); + $endpoint->add('234567890123456789', 'bob'); + self::assertSame(['alice', 'bob'], array_column($state->getVerifyList(), 'ss14')); + self::assertSame('bob', $endpoint->fetch('discord', '234567890123456789')['ss14']); + } +}