diff --git a/server/src/services/chatgenie.ts b/server/src/services/chatgenie.ts index f06ffdf..757d026 100644 --- a/server/src/services/chatgenie.ts +++ b/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 ` 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(); +// trailing half-line from the last read, waiting for its newline (see readNewLines) +const partials = new Map(); // 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 "]: 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: 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 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 { @@ -2330,7 +2385,7 @@ async function tick(log: (msg: string) => void): Promise { 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 { 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, " ]: 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;