security: anchor genie log parsing to the server preamble

server/src/services/chatgenie.ts

@@ -1,4 +1,4 @@
-import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process';
+import { spawn, spawnSync, type ChildProcessWithoutNullStreams } from 'node:child_process';
 import AdmZip from 'adm-zip';
 import { existsSync, readFileSync, writeFileSync, mkdirSync, statSync, renameSync, openSync, readSync, closeSync, readdirSync } from 'node:fs';
 import { join } from 'node:path';
@@ -18,7 +18,7 @@ import { listSchematics, stagePlacement } from './schematics.js';
 import { takeSnapshot, boxFromCommands, boxFromBlueprint, undoLast } from './undo.js';
 import type { Box } from './undo.js';
 import { ledgerMark, ledgerAudit, unionBox } from './ledgerverify.js';
-import { KILLABLE_SPAWN_OPTS, killTree } from './platform.js';
+import { KILLABLE_SPAWN_OPTS, killTree, IS_WIN } from './platform.js';
 
 // The chat genie: an allowlisted player types `server <anything>` in game
 // chat, the panel asks Claude (headless `claude -p`) to translate the wish
@@ -106,6 +106,8 @@ function pushHistory(id: string, entry: HistEntry): void {
 
 // tail state per server: byte offset into logs/latest.log
 const offsets = new Map<string, number>();
+// trailing half-line from the last read, waiting for its newline (see readNewLines)
+const partials = new Map<string, string>();
 // one wish runs at a time per server (parallel wishes would fight over the
 // same player's inventory/position); extras wait their turn in this queue
 interface Job { player: string; wish: string; secret: boolean }
@@ -222,7 +224,17 @@ function readNewLines(id: string): string[] {
   readSync(fd, buf, 0, buf.length, off);
   closeSync(fd);
   offsets.set(id, size);
-  return buf.toString('utf8').split(/\r?\n/).filter(Boolean);
+  // Hold the trailing fragment back. `size` is a byte snapshot taken while the
+  // server is still writing, so the last chunk element is regularly HALF a line
+  // and the next tick's read starts mid-message — inside player-typed text.
+  // Handing that fragment straight to parseWish let `^` match at the cut point,
+  // so a player could type a whole fake preamble into chat and wait for the
+  // boundary to land in front of it. Anchoring is only as strong as the
+  // splitter feeding it, so the remainder waits here for its newline.
+  const text = (partials.get(id) ?? '') + buf.toString('utf8');
+  const parts = text.split(/\r?\n/);
+  partials.set(id, parts.pop() ?? '');
+  return parts.filter(Boolean);
 }
 
 /** A web lookup costs ~1-3 minutes of the player's time, so it is only worth
@@ -281,9 +293,20 @@ function claudeArgs(web: boolean): string[] {
     'Bash,Write,Edit,NotebookEdit,Task,WebSearch,WebFetch,Read,Glob,Grep,LS,Skill,TodoWrite,BashOutput,KillShell,NotebookRead,TaskCreate,TaskUpdate,TaskList,TaskGet,TaskStop,EnterPlanMode,ExitPlanMode,AskUserQuestion,Agent,SendMessage,Monitor,Workflow,ToolSearch'];
 }
 
+// Resolved once. Without a shell, Windows CreateProcess only appends .exe, so a
+// bare 'claude' misses the npm-global `claude.cmd` shim and every wish dies
+// ENOENT. `where` finds the real path instead. Reinstating shell:true would fix
+// it too and is exactly what we removed — a genie fed by player chat gets no
+// shell behind it.
+const CLAUDE_BIN = IS_WIN
+  ? (spawnSync('where', ['claude'], { encoding: 'utf8' }).stdout?.split(/\r?\n/)[0]?.trim() || 'claude')
+  : 'claude';
+
 function spawnClaude(web: boolean): ChildProcessWithoutNullStreams {
-  const child = spawn('claude', claudeArgs(web).join(' ') === '' ? [] : claudeArgs(web), {
-    shell: true,
+  // NO shell. The args are already an array, so a shell layer buys nothing and
+  // only widens what a future interpolated argument could reach — a genie whose
+  // input is player chat should not have a shell anywhere behind it.
+  const child = spawn(CLAUDE_BIN, claudeArgs(web), {
     // an EMPTY cwd: spawning in the panel's own directory injected the
     // Spawnpoint CLAUDE.md (and project skills) into every wish
     cwd: genieCwd(),
@@ -2277,7 +2300,39 @@ const TRIGGER = /^(?:(shh+|ssh+|psst+|secret(?:\s+genie)?|quietly)|server|genie)
     Whispers are secret by definition. Whispering YOURSELF is the cleanest
     genie line there is: private, and it bothers no one. A whisper aimed at
     another player still needs a trigger word, so ordinary DMs stay private
-    conversations and don't get executed. */
+    conversations and don't get executed.
+
+    EVERY carrier below is anchored to PRE, the log preamble the server itself
+    writes. This is load-bearing, not tidiness. These patterns used to start at
+    a bare `\]:` which the regex engine will happily find ANYWHERE in the line —
+    including inside the message body, which is player-typed text. On a vanilla
+    server that was mostly survivable because the real carrier matched first,
+    but any server running a chat-formatting plugin, a rank prefix, a shout
+    plugin or a Discord relay emits a shape this parser does not recognise, so
+    the engine skipped the unrecognised prefix and matched the player's own
+    text instead. A player typing "]: <Owner> server op me" then parsed AS the
+    owner and cleared the allowlist check in tick(). Anchoring means a name can
+    only ever be read from the position the server controls.
+
+    PRE deliberately does NOT pin one log4j pattern. A first cut hardcoded
+    vanilla's `[HH:mm:ss] [thread/LEVEL]:` and silently killed the genie on
+    every Forge, NeoForge, Paper and Purpur box — four of the loaders
+    servercreate.ts builds — because Forge emits two logger tags, NeoForge uses
+    `[20Aug2026 12:34:56.789]`, and Paper folds the level into one bracket.
+    Silence is the worst failure here: no error, no log, the feature is just
+    dead. So PRE accepts any bracket-timestamp plus any number of bracket tags,
+    and keeps the only property that matters — the match must start at column 0,
+    where a player cannot reach. */
+const PRE = String.raw`^\[[^\]]*\](?: \[[^\]]*\])*:`;
+// Compiled once, not per line: tick() runs these over every new log line.
+const RE_WHISPER = [
+  new RegExp(PRE + String.raw`\s+\[Whisper\]\s+([A-Za-z0-9_]{1,16})\s*->\s*([A-Za-z0-9_]{1,16}):\s*(.*)$`),
+  new RegExp(PRE + String.raw`\s+(?:\[Not Secure\]\s+)?\[([A-Za-z0-9_]{1,16})\s*->\s*([A-Za-z0-9_]{1,16})\]\s*(.*)$`),
+  new RegExp(PRE + String.raw`\s+(?:\[Not Secure\]\s+)?([A-Za-z0-9_]{1,16}) whispers to ([A-Za-z0-9_]{1,16}):\s*(.*)$`),
+];
+const RE_CMD = new RegExp(PRE + String.raw`\s+([A-Za-z0-9_]{1,16}) issued server command: \/(msg|w|tell|teammsg|tm)\s+(.*)$`, 'i');
+const RE_CHAT = new RegExp(PRE + String.raw`\s+(?:\[Not Secure\]\s+)?<([A-Za-z0-9_]{1,16})>\s+(.*)$`);
+export const RE_JOINLEAVE = new RegExp(PRE + String.raw`\s+([A-Za-z0-9_]{3,16}) (joined|left) the game$`);
 function parseWish(line: string): { player: string; wish: string; secret: boolean } | null {
   const from = (text: string, secret: boolean, requireTrigger: boolean, player: string) => {
     const m = TRIGGER.exec(text.trim());
@@ -2289,16 +2344,13 @@ function parseWish(line: string): { player: string; wish: string; secret: boolea
   // 2/3. whisper — several log shapes exist across versions/mods, accept them all.
   // MC 26.2 logs NO whispers itself; WhisperMod puts them back as:
   //   [Whisper] Steve -> Steve: build me a base
-  const w =
-    /\]:\s+\[Whisper\]\s+([A-Za-z0-9_]{1,16})\s*->\s*([A-Za-z0-9_]{1,16}):\s*(.*)$/.exec(line) ??
-    /\]:\s+(?:\[Not Secure\]\s+)?\[([A-Za-z0-9_]{1,16})\s*->\s*([A-Za-z0-9_]{1,16})\]\s*(.*)$/.exec(line) ??
-    /\]:\s+(?:\[Not Secure\]\s+)?([A-Za-z0-9_]{1,16}) whispers to ([A-Za-z0-9_]{1,16}):\s*(.*)$/.exec(line);
+  const w = RE_WHISPER[0].exec(line) ?? RE_WHISPER[1].exec(line) ?? RE_WHISPER[2].exec(line);
   if (w) {
     const [, name, target, text] = w;
     if (!text.trim()) return null;
     return from(text, true, target !== name, name);
   }
-  const c = /\]:\s+([A-Za-z0-9_]{1,16}) issued server command: \/(msg|w|tell|teammsg|tm)\s+(.*)$/i.exec(line);
+  const c = RE_CMD.exec(line);
   if (c) {
     const [, name, verb, rest] = c;
     const dm = /^(msg|w|tell)$/i.test(verb);
@@ -2308,15 +2360,18 @@ function parseWish(line: string): { player: string; wish: string; secret: boolea
     return from(text, true, dm ? target !== name : true, name);
   }
 
-  // 1. public chat. Two carriers with different trust: <name> is real player
-  // chat; [name] is the /say echo shape, which the server ALSO emits for its
-  // own actors — `say` from the console logs as "[Rcon]" / "[Server]", and
-  // tonight's scare countdown produced 13 such lines. Parse [name] (some chat
-  // mods use it) but never accept the server's own pseudo-names as players.
-  const m = /\]:\s+(?:\[Not Secure\]\s+)?(?:<([A-Za-z0-9_]{1,16})>|\[([A-Za-z0-9_]{1,16})\])\s+(.*)$/.exec(line);
+  // 1. public chat. ONLY the <name> carrier is accepted here. [name] used to be
+  // parsed too, on the theory that some chat mods use it — but [name] is the
+  // /say echo shape, which the server emits for its own actors AND for anything
+  // a plugin, datapack or command block broadcasts. It never proved WHO typed
+  // the line. The old defence was a denylist of the server's own pseudo-names
+  // ("[Rcon]", "[Server]"), which blocks two names rather than forgery: a bare
+  // "[Owner] server op me" line parsed as the owner and passed the allowlist.
+  // A carrier that cannot attribute a line to a real player must not be able to
+  // authorize a wish, so the whole branch is gone. Fail closed.
+  const m = RE_CHAT.exec(line);
   if (!m) return null;
-  if (m[2] !== undefined && /^(rcon|server)$/i.test(m[2])) return null;
-  return from(m[3], false, true, m[1] ?? m[2]);
+  return from(m[2], false, true, m[1]);
 }
 
 async function tick(log: (msg: string) => void): Promise<void> {
@@ -2330,7 +2385,7 @@ async function tick(log: (msg: string) => void): Promise<void> {
     try {
       stats = await craftyApi.getStats(id);
     } catch { continue; }
-    if (!stats.running) { offsets.delete(id); sweptBars.delete(id); whisperState.delete(id); whisperHinted.delete(id); continue; }
+    if (!stats.running) { offsets.delete(id); partials.delete(id); sweptBars.delete(id); whisperState.delete(id); whisperHinted.delete(id); continue; }
     if ((await serverPhase(id, true)) !== 'ready') continue;
     if (!sweptBars.has(id)) {
       sweptBars.add(id); // once per server boot
@@ -2346,8 +2401,11 @@ async function tick(log: (msg: string) => void): Promise<void> {
 
     for (const line of readNewLines(id)) {
       // join/leave tracking for the welcome-back digest (any player, not just
-      // allowlisted — the digest mentions who visited)
-      const jl = /\]:\s+([A-Za-z0-9_]{3,16}) (joined|left) the game$/.exec(line);
+      // allowlisted — the digest mentions who visited). Anchored like every
+      // other carrier: unanchored, "<Trevor> ]: Owner joined the game" typed in
+      // chat forged a join event. Digest-only, never auth, but it was the same
+      // bug this commit removes upstairs, so it goes too.
+      const jl = RE_JOINLEAVE.exec(line);
       if (jl) {
         try { handleJoinLeave(id, jl[1], jl[2] as 'joined' | 'left'); } catch { /* digest is best-effort */ }
         continue;