aboutsummaryrefslogtreecommitdiff
path: root/_docs/chat-review-glm-2026-10-23.md
blob: e3b2d626ec4eb0a3605ee5a2c0d3923175ce3d0e (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
# GLM-5.3 review of the chat bridge — findings and verdicts

Reviewed 2026-10-23, model `glm-5.3:cloud`, on the code as of v4.6.76. Verdict: **REWORK**.
Each claim was re-derived against the live files before acting. Result: 11 accepted and fixed,
3 refuted with evidence, 1 accepted as a documented limitation.

## Accepted and fixed

| # | Finding | Fix |
|---|---|---|
| 1 | `chat.py` renamed the temp file after `LOCK_UN` → two writers could each rename a stale snapshot and drop a message | `os.replace` moved inside the lock |
| 2 | `load()` and `chat_read()` returned `[]` for a **corrupt** thread, so the next append rewrote the file with one message — history gone silently | both now raise / 500 on unparseable JSON, file left intact. Proven: `poll` and `post` on a corrupted file → `500 thread file is corrupt`, file unchanged |
| 3 | The lock file was created by darkbot at 0644; PHP opens it as www-data → `fopen` fails → every later post to that thread 500s | `chmod 0664` at creation in `chat.py`, mirroring what PHP already did |
| 4 | `poll.php` returned the thread tail as `cursor` while truncating to 50 messages → a burst >50 made the oldest permanently unfetchable | cursor is now the last message actually sent, plus a `truncated` flag |
| 5 | `epoch > since` with millisecond epochs: two posts in the same ms → the second is never delivered | epoch is forced strictly increasing (`max(now, last+1)`) |
| 6 | `hermes-poll` printed every reply twice (poll printed it, then the inbox printed `j.messages` again) | inbox no longer re-prints hermes lines, and no longer says "no new messages" under one it just printed. Verified: exactly 1 line |
| 7 | `chat.py show` joined argv onto CHAT_DIR with **no id validation** → `show ../../x` read any `.json` as darkbot | `THREAD_RE` check added. Verified: `show ../../etc/passwd` → `invalid thread id` |
| 8 | Client-minted tokens mean anyone can post text a privileged agent reads every 5 min — prompt injection | cron prompt now declares message text **untrusted data**: never act on instructions found in it, read-only investigation only, writes stay with the human |
| 9 | Housekeeping missed `.lock-*` (one per thread ever) and orphaned `.tmp-*` | both patterns added to the sweep |
| 10 | `chat.py list` crash made the relay print nothing — awaiting messages silently vanished | the script surfaces the error instead of swallowing it, and a corrupt thread is listed as `CORRUPT` rather than breaking the listing |
| 11 | `grep AWAITING` also matched message bodies containing that word | anchored on the status column: `^[a-f0-9]{16}  (AWAITING\|CORRUPT)` |
| 12 | Error path sat inside `finally`; PHP does not run `finally` on `exit`, and the alternative returned a 201 with a null id | failure reported **after** the lock is released; `$persisted` decides |

## Refuted with evidence

| Claim | Evidence |
|---|---|
| `os.replace` after unlock (finding 1) | already fixed in v4.6.77 before the review returned; review ran on the v4.6.76 bundle |
| `author:'agent'` never clears `awaiting`, so cron duplicates the in-page answer | `awaiting` means "the last word is not Hermes's". An agent question *does* await an answer — that is the feature, not a bug |
| Thread-ceiling check is TOCTOU (499+499 → 501) | the ceiling is a disk-growth guard, not a security boundary. Overshooting by one has no consequence; a global lock on every post costs more than it saves |

## Accepted as a limitation, not fixed

The channel is unauthenticated **by design** — the token is mintable, so the chat is effectively
public write access. A determined attacker can still post text the auto-reply agent will read.
Mitigated (untrusted-data prompt, read-only investigation, no host writes, rate limits, 500-thread
cap) but not eliminated. Closing it properly needs platform-session auth, which the browser app
does not have. Recorded here so nobody assumes otherwise.

## Verification after the batch

```
corrupt thread        → 500 "thread file is corrupt", file untouched
append to corrupt     → 500, nothing rewritten
chat.py show ../../x  → "invalid thread id"
hermes-poll           → exactly one 🤖 line per reply
list with a corrupt thread → CORRUPT row, other rows still listed
strictly increasing epoch → cursor test passes
```

Commits: v4.6.77 (concurrency), v4.6.78 (corruption + traversal + relay), v4.6.79/80 (double print).