From 4d717018ea8a748b8bb16ad71c3deab6d8c20545 Mon Sep 17 00:00:00 2001 From: Agando Date: Fri, 23 Oct 2026 20:37:54 +0200 Subject: fix(chat): GLM review batch — corruption, traversal, cursor truncation, double print (v4.6.78-80) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 12 findings accepted, 3 refuted with evidence (see _docs/chat-review-glm-2026-10-23.md). Data loss: a corrupt thread read as [] and the next append rewrote it with one message — now 500 and the file is left alone. Cursor after a >50-message truncation pointed at the thread tail, making the oldest permanently unfetchable — now the last message actually sent, plus truncated:true. Two posts in the same millisecond shared a cursor — epoch is now strictly increasing. Security: chat.py show() had no id validation and joined argv onto CHAT_DIR ('../../x' read any .json as darkbot) — THREAD_RE added. The auto-reply prompt now treats message text as untrusted data and forbids acting on instructions found in it. Ops: relay housekeeping swept neither .lock-* nor orphaned .tmp-*; a crashed listing looked identical to an empty queue; grep AWAITING also matched message bodies; hermes-poll printed every reply twice. Verified: corrupt file -> 500 with the file untouched; traversal -> 'invalid thread id'; hermes-poll -> exactly one line per reply. --- api/chat/lib.php | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) (limited to 'api/chat/lib.php') diff --git a/api/chat/lib.php b/api/chat/lib.php index 6a0f923..94afe94 100644 --- a/api/chat/lib.php +++ b/api/chat/lib.php @@ -51,8 +51,15 @@ function chat_path(string $thread): string { } function chat_read(string $thread): array { - $all = load_json(chat_path($thread)); - return is_array($all) ? $all : []; + $path = chat_path($thread); + if (!is_file($path)) return []; + $raw = @file_get_contents($path); + if ($raw === false) error_response('thread is unreadable', 500); + $all = json_decode($raw, true); + // A corrupt file must NOT read as "empty": the next append would then rewrite the + // thread with a single message and the history is gone for good. + if (!is_array($all)) error_response('thread file is corrupt: ' . basename($path), 500); + return $all; } // Atomic replace: a reader sees the old file or the new one, never a truncated one. @@ -83,12 +90,17 @@ function chat_append(string $thread, string $author, string $text): array { error_response('failed to lock thread', 500); } $msg = []; + $persisted = false; try { $messages = chat_read($thread); + $lastEpoch = $messages ? (int)($messages[count($messages) - 1]['epoch'] ?? 0) : 0; + $now = (int)round(microtime(true) * 1000); $msg = [ 'id' => 'MSG-' . gmdate('Ymd-His') . '-' . bin2hex(random_bytes(3)), 'ts' => gmdate('c'), // UTC, always — never a local offset - 'epoch' => (int)round(microtime(true) * 1000), // ms — second precision drops same-second messages + // strictly increasing: two posts in the same millisecond would otherwise share a + // cursor value and the second would never be delivered to a `since` reader + 'epoch' => max($now, $lastEpoch + 1), 'author' => $author, 'text' => $text, ]; @@ -101,13 +113,14 @@ function chat_append(string $thread, string $author, string $text): array { if (count($messages) > CHAT_MAX_MESSAGES) { $messages = array_slice($messages, -CHAT_MAX_MESSAGES); } - if (!chat_write($thread, $messages)) { - error_response('failed to persist message', 500); - } + $persisted = chat_write($thread, $messages); } finally { flock($lock, LOCK_UN); fclose($lock); } + // error_response() exits, and PHP does not run finally blocks on exit — so the + // failure must be reported after the lock is released, never from inside it. + if (!$persisted) error_response('failed to persist message', 500); return $msg; } -- cgit v1.2.3