fix(session): validate ids and repair deletion

Validate the id extracted from ttrpg/+/stream and ttrpg/+/meta before it is used to build a path, closing a traversal where a .. topic level (or ..\.. on Windows) could escape the sessions directory and delete the whole data root. Meta writes now go through the schema, so malformed payloads are ignored instead of deleting.

Deletion was also a no-op: an empty payload threw in JSON.parse before the delete branch ran, so sessions were never removed. Check for an empty/null payload before parsing. The worker validates ids and surfaces publish/connection failures.

src/cli/journal.test.ts

@@ -11,6 +11,8 @@ import { tmpdir } from "os";
 import { join } from "path";
 import {
   appendStream,
+  extractSessionId,
+  handleMetaChange,
   metaPath,
   migrateLegacyManifest,
   repairStreamTail,
@@ -18,6 +20,7 @@ import {
   sessionDir,
   streamPath,
   writeFileAtomic,
+  type SessionMeta,
 } from "./journal";
 
 function makeRoot(): string {
@@ -140,4 +143,137 @@ describe("appendStream", () => {
     expect(readFileSync(streamPath(root, "known"), "utf-8")).toBe('{"x":1}\n');
     cleanup(root);
   });
+
+  test("refuses to build a path from a malformed id", () => {
+    const root = makeRoot();
+    mkdirSync(join(root, "sessions"), { recursive: true });
+    const index: Record<string, SessionMeta> = {
+      "..": { name: "evil", created: 1, players: [] },
+    };
+
+    appendStream(root, "..", index, '{"x":1}');
+
+    expect(existsSync(join(root, "stream.jsonl"))).toBe(false);
+    cleanup(root);
+  });
+});
+
+describe("extractSessionId", () => {
+  test("returns a valid id from a session topic", () => {
+    expect(extractSessionId("ttrpg/my-game/stream")).toBe("my-game");
+    expect(extractSessionId("ttrpg/A_1/meta")).toBe("A_1");
+  });
+
+  test("rejects traversal, empty and malformed ids", () => {
+    expect(extractSessionId("ttrpg/../meta")).toBeNull();
+    expect(extractSessionId("ttrpg/./meta")).toBeNull();
+    expect(extractSessionId("ttrpg/..\\..\\evil/meta")).toBeNull();
+    expect(extractSessionId("ttrpg/a b/meta")).toBeNull();
+    expect(extractSessionId("ttrpg//meta")).toBeNull();
+    expect(extractSessionId("ttrpg/meta")).toBeNull();
+  });
+});
+
+describe("handleMetaChange", () => {
+  // Minimal broker stub: handleMetaChange only needs publish() for the
+  // manifest broadcast.
+  function makeBroker() {
+    const published: { topic: string; payload: string }[] = [];
+    const broker = {
+      publish: (
+        packet: { topic: string; payload: Buffer },
+        cb?: () => void,
+      ) => {
+        published.push({
+          topic: packet.topic,
+          payload: packet.payload.toString(),
+        });
+        cb?.();
+      },
+    } as unknown as import("aedes").Aedes;
+    return { broker, published };
+  }
+
+  test("writes validated meta, updates the index and publishes the manifest", () => {
+    const root = makeRoot();
+    const index: Record<string, SessionMeta> = {};
+    const { broker, published } = makeBroker();
+
+    handleMetaChange(
+      root,
+      "game",
+      broker,
+      index,
+      JSON.stringify({ name: "Game", created: 10, players: ["alice"] }),
+    );
+
+    expect(index["game"]).toEqual({
+      name: "Game",
+      created: 10,
+      players: ["alice"],
+    });
+    expect(readFileSync(metaPath(root, "game"), "utf-8")).toContain("Game");
+    expect(published.some((p) => p.topic === "ttrpg/$SESSIONS")).toBe(true);
+    cleanup(root);
+  });
+
+  test("deletes a session on an empty payload", () => {
+    const root = makeRoot();
+    mkdirSync(sessionDir(root, "doomed"), { recursive: true });
+    writeFileAtomic(
+      metaPath(root, "doomed"),
+      JSON.stringify({ name: "Doomed", created: 1, players: [] }),
+    );
+    const index: Record<string, SessionMeta> = {
+      doomed: { name: "Doomed", created: 1, players: [] },
+    };
+    const { broker } = makeBroker();
+
+    handleMetaChange(root, "doomed", broker, index, "");
+
+    expect(index["doomed"]).toBeUndefined();
+    expect(existsSync(sessionDir(root, "doomed"))).toBe(false);
+    cleanup(root);
+  });
+
+  test("refuses malformed ids and never touches the filesystem", () => {
+    const root = makeRoot();
+    mkdirSync(join(root, "sessions"), { recursive: true });
+    const index: Record<string, SessionMeta> = {};
+    const { broker } = makeBroker();
+
+    handleMetaChange(root, "..", broker, index, JSON.stringify({ name: "evil" }));
+    handleMetaChange(root, ".", broker, index, "");
+    handleMetaChange(root, "..\\..\\evil", broker, index, "");
+
+    expect(existsSync(join(root, "meta.json"))).toBe(false);
+    expect(existsSync(join(root, "sessions", "meta.json"))).toBe(false);
+    expect(Object.keys(index)).toHaveLength(0);
+    cleanup(root);
+  });
+
+  test("ignores malformed meta without deleting the session", () => {
+    const root = makeRoot();
+    mkdirSync(sessionDir(root, "keep"), { recursive: true });
+    writeFileAtomic(
+      metaPath(root, "keep"),
+      JSON.stringify({ name: "Keep", created: 1, players: [] }),
+    );
+    const index: Record<string, SessionMeta> = {
+      keep: { name: "Keep", created: 1, players: [] },
+    };
+    const { broker } = makeBroker();
+
+    handleMetaChange(
+      root,
+      "keep",
+      broker,
+      index,
+      JSON.stringify({ players: "not-an-array" }),
+    );
+
+    expect(index["keep"]).toEqual({ name: "Keep", created: 1, players: [] });
+    expect(existsSync(metaPath(root, "keep"))).toBe(true);
+    cleanup(root);
+  });
 });

src/cli/journal.ts

@@ -15,6 +15,9 @@
  * 会话列表在内存中派生并通过保留主题 `ttrpg/$SESSIONS` 发布。
  * 元数据写入使用「临时文件 + fsync + 原子重命名」，避免崩溃时
  * 截断损坏；损坏的元数据会被隔离而非静默丢弃。
+ *
+ * 会话 ID 会直接决定文件系统路径，因此在 MQTT 边界严格校验
+ * （见 shared/session-id）。
  */
 
 import { join, resolve } from "path";
@@ -37,6 +40,7 @@ import {
 import type { Server as HttpServer } from "http";
 import { WebSocketServer, createWebSocketStream } from "ws";
 import { z } from "zod";
+import { isValidSessionId } from "../shared/session-id.js";
 
 // ---------------------------------------------------------------------------
 // 类型定义
@@ -50,6 +54,8 @@ const SessionMetaSchema = z.object({
 
 type SessionMeta = z.infer<typeof SessionMetaSchema>;
 
+export type { SessionMeta };
+
 interface SessionManifest {
   sessions: Record<string, SessionMeta>;
 }
@@ -167,6 +173,10 @@ export function scanSessions(dataRoot: string): Record<string, SessionMeta> {
   for (const entry of readdirSync(sessionsRoot, { withFileTypes: true })) {
     if (!entry.isDirectory()) continue;
     const id = entry.name;
+    if (!isValidSessionId(id)) {
+      console.warn(`[日志] 跳过非法会话目录：${id}`);
+      continue;
+    }
     const mp = metaPath(dataRoot, id);
     const sp = streamPath(dataRoot, id);
 
@@ -227,6 +237,10 @@ export function migrateLegacyManifest(dataRoot: string): void {
     let migrated = 0;
 
     for (const [id, raw] of Object.entries(sessions)) {
+      if (!isValidSessionId(id)) {
+        console.warn(`[日志] 跳过非法会话 ID：${id}`);
+        continue;
+      }
       const mp = metaPath(dataRoot, id);
       if (existsSync(mp)) continue;
       try {
@@ -361,9 +375,15 @@ export async function createJournalServer(
 // 工具函数
 // ---------------------------------------------------------------------------
 
-function extractSessionId(topic: string): string | null {
+export function extractSessionId(topic: string): string | null {
   const parts = topic.split("/");
-  return parts.length >= 3 ? parts[1] : null;
+  if (parts.length < 3) return null;
+  const id = parts[1];
+  if (!isValidSessionId(id)) {
+    console.warn(`[日志] 拒绝非法会话 ID：${JSON.stringify(id)}`);
+    return null;
+  }
+  return id;
 }
 
 // ---------------------------------------------------------------------------
@@ -381,42 +401,65 @@ export function appendStream(
   index: Record<string, SessionMeta>,
   line: string,
 ): void {
-  if (!index[id]) {
+  if (!isValidSessionId(id) || !index[id]) {
     console.warn(`[日志] 丢弃未知会话 ${id} 的流记录`);
     return;
   }
   appendFileSync(streamPath(root, id), line + "\n", "utf-8");
 }
 
-function handleMetaChange(
+export function handleMetaChange(
   root: string,
   sessionId: string,
   broker: import("aedes").Aedes,
   index: Record<string, SessionMeta>,
   rawPayload: string,
 ): void {
-  try {
+  // The id reaches us straight from an MQTT topic level; never build a path
-    const raw = JSON.parse(rawPayload) as Partial<SessionMeta> | null;
+  // from it without validating, or `..` can escape the sessions directory.
+  if (!isValidSessionId(sessionId)) {
+    console.warn(
+      `[日志] 拒绝非法会话 ID 的元数据操作：${JSON.stringify(sessionId)}`,
+    );
+    return;
+  }
 
-    if (!raw || !raw.name) {
+  const payload = rawPayload.trim();
-      // 空载荷 = 删除会话：递归删除目录，移除索引项
+
+  // 空载荷（或 JSON null）= 删除会话：递归删除目录，移除索引项。
+  // 必须在 JSON.parse 之前判断：JSON.parse("") 会抛错，导致删除永远无法生效。
+  if (payload === "" || payload === "null") {
     delete index[sessionId];
     rmSync(sessionDir(root, sessionId), { recursive: true, force: true });
     console.log(`[日志] 会话已删除：${sessionId}`);
-    } else {
+    publishManifest(broker, index);
-      const meta: SessionMeta = {
+    return;
-        name: raw.name,
+  }
-        created: raw.created || Date.now(),
+
-        players: raw.players || [],
+  let raw: unknown;
-      };
+  try {
+    raw = JSON.parse(payload);
+  } catch (e) {
+    console.error(`[日志] 会话 ${sessionId} 元数据解析失败：`, e);
+    return;
+  }
+
+  // 用 schema 校验，避免写入无法回读的数据（例如非数组的 players），
+  // 那会让下次启动扫描时把元数据隔离掉。
+  const parsed = SessionMetaSchema.safeParse(raw);
+  if (!parsed.success) {
+    console.error(`[日志] 会话 ${sessionId} 元数据无效，已忽略：`, parsed.error);
+    return;
+  }
+
+  const meta = parsed.data;
+  try {
     mkdirSync(sessionDir(root, sessionId), { recursive: true });
     writeFileAtomic(metaPath(root, sessionId), JSON.stringify(meta, null, 2));
     index[sessionId] = meta;
     console.log(`[日志] 会话已更新：${sessionId} (${meta.name})`);
-    }
-
     publishManifest(broker, index);
   } catch (e) {
-    console.error(`[日志] 会话 ${sessionId} 元数据解析失败：`, e);
+    console.error(`[日志] 会话 ${sessionId} 元数据写入失败：`, e);
   }
 }

src/workers/journal.worker.ts

@@ -12,6 +12,11 @@
 
 import * as Comlink from "comlink";
 import type { StreamMessage } from "../components/journal/registry";
+import {
+  SESSION_ID_MAX_LENGTH,
+  isValidSessionId,
+  toSessionId,
+} from "../shared/session-id.js";
 
 // ---------------------------------------------------------------------------
 // Types
@@ -311,6 +316,10 @@ async function joinSession(
   name?: string,
   role?: "gm" | "player" | "observer",
 ): Promise<void> {
+  if (!isValidSessionId(sessionId)) {
+    throw new Error(`Invalid session id: ${JSON.stringify(sessionId)}`);
+  }
+
   const client = _mqttClient;
   if (!client || !_mqttConnected) {
     throw new Error("Not connected to broker");
@@ -493,17 +502,29 @@ function disconnectStream(): void {
 // Session lifecycle
 // ---------------------------------------------------------------------------
 
+/**
+ * Avoid silently overwriting an existing session whose name slugifies to the
+ * same id (e.g. "My Game" and "my game"). Best-effort: the retained manifest
+ * may not have arrived yet.
+ */
+function uniqueSessionId(base: string): string {
+  if (!sessionList.sessions[base]) return base;
+  const stem = base.slice(0, SESSION_ID_MAX_LENGTH - 5);
+  for (let n = 2; n < 1000; n++) {
+    const candidate = `${stem}-${n}`;
+    if (!sessionList.sessions[candidate]) return candidate;
+  }
+  return `${stem}-${Date.now()}`.slice(0, SESSION_ID_MAX_LENGTH);
+}
+
 function createSession(
   name: string,
   players: string[] = [],
 ): Promise<string | null> {
   if (!_mqttClient || !_mqttConnected) return Promise.resolve(null);
 
-  const id =
+  const id = uniqueSessionId(toSessionId(name));
-    name
+  if (!isValidSessionId(id)) return Promise.resolve(null);
-      .toLowerCase()
-      .replace(/[^a-z0-9]+/g, "-")
-      .replace(/^-|-$/g, "") || "session";
 
   const meta: SessionMeta = { name, created: Date.now(), players };
 
@@ -525,13 +546,22 @@ function createSession(
 }
 
 function deleteSession(sessionId: string): Promise<void> {
-  if (!_mqttClient || !_mqttConnected) return Promise.resolve();
+  if (!isValidSessionId(sessionId)) {
-  return new Promise((resolve) => {
+    return Promise.reject(
+      new Error(`Invalid session id: ${JSON.stringify(sessionId)}`),
+    );
+  }
+  if (!_mqttClient || !_mqttConnected) {
+    return Promise.reject(new Error("Not connected to stream"));
+  }
+  return new Promise((resolve, reject) => {
+    // Empty retained payload: the broker clears the retained meta and the
+    // server treats the empty payload as a delete.
     _mqttClient!.publish(
       `ttrpg/${sessionId}/meta`,
       "",
       { qos: 1, retain: true },
-      () => resolve(),
+      (err) => (err ? reject(err) : resolve()),
     );
   });
 }