From 9ecd497f1e61653d03b803406c6dfff022271792 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sun, 18 Oct 2026 19:53:36 +0000 Subject: [PATCH] Reserve the deleted marker for tombstones Imports and uploads could carry deleted: false, which the loader took for a tombstone on the next restart. The schemas now reject the property, and the loader only treats deleted: true as a tombstone and strips the key from entries already saved with it. Co-Authored-By: Claude Opus 5.5 --- plugins/player_sync/controller.ts | 6 +++-- plugins/player_sync/messages.ts | 4 ++++ plugins/player_sync/test/controller.js | 32 ++++++++++++++++++-------- plugins/player_sync/test/plugin.js | 10 ++++++++ 4 files changed, 40 insertions(+), 12 deletions(-) diff --git a/plugins/player_sync/controller.ts b/plugins/player_sync/controller.ts index 2938ffa9..419c1e0e 100644 --- a/plugins/player_sync/controller.ts +++ b/plugins/player_sync/controller.ts @@ -41,10 +41,12 @@ async function loadDatabase(config: lib.ControllerConfig, logger: lib.Logger) { let playerDatastore = new Map(); let tombstones = new Map(); for (let [name, entry] of entries) { - if ("deleted" in entry) { + if ("deleted" in entry && entry.deleted === true) { tombstones.set(name, entry); } else { - playerDatastore.set(name, entry); + // Imports used to keep the marker + delete (entry as any).deleted; + playerDatastore.set(name, entry as IpcPlayerData); } } return { playerDatastore, tombstones }; diff --git a/plugins/player_sync/messages.ts b/plugins/player_sync/messages.ts index 6799e41c..ea780235 100644 --- a/plugins/player_sync/messages.ts +++ b/plugins/player_sync/messages.ts @@ -31,10 +31,13 @@ export type IpcPlayerData = { components?: Record | unknown[], } +// Marks tombstones in player_sync.json +const jsonReserved = { deleted: Type.Optional(Type.Never()) }; const jsonPlayerData = Type.Object({ generation: Type.Integer({ minimum: 0 }), epoch: Type.Optional(Type.Integer({ minimum: 0 })), name: Type.String(), + ...jsonReserved, }); // The parts of an import that module/serialize.lua relies on, item stacks are checked by the Lua deserializer @@ -150,6 +153,7 @@ const jsonImportedPlayerData = Type.Object({ crafting_queue: Type.Optional(jsonCraftingQueue), recipe_notifications: Type.Optional(Type.String()), components: Type.Optional(jsonTable), + ...jsonReserved, }); export class AcquireResponse { diff --git a/plugins/player_sync/test/controller.js b/plugins/player_sync/test/controller.js index 4177bdf1..eb44d056 100644 --- a/plugins/player_sync/test/controller.js +++ b/plugins/player_sync/test/controller.js @@ -208,23 +208,35 @@ describe("player_sync controller", function() { await assert.rejects(set("stored", playerData("stored", 3)), acquiredError); assert.equal((await saved()).get("stored").generation, 3); }); + + // The link validates requests before the handler runs + const fromJSON = lib.Link.requestFromJSON(msg.SetPlayerDataRequest, "SetPlayerDataRequest"); + async function setJSON(name, data) { + const request = fromJSON({ playerName: name, playerData: data }); + return await handler(msg.SetPlayerDataRequest)(request); + } it("should keep the stored data when an import fails validation", async function() { - // The link validates requests before the handler runs - const fromJSON = lib.Link.requestFromJSON(msg.SetPlayerDataRequest, "SetPlayerDataRequest"); - async function setJSON(name, data) { - const request = fromJSON({ playerName: name, playerData: data }); - return await handler(msg.SetPlayerDataRequest)(request); - } await release("stored"); await setJSON("stored", playerData("stored", 0)); const before = (await saved()).get("stored"); assert.equal(before.epoch, 1); - await assert.rejects( - setJSON("stored", playerData("stored", 0, { crafting_queue: {} })), - { message: "Request SetPlayerDataRequest failed validation" }, - ); + for (const extra of [{ crafting_queue: {} }, { deleted: false }, { deleted: true }]) { + await assert.rejects( + setJSON("stored", playerData("stored", 0, extra)), + { message: "Request SetPlayerDataRequest failed validation" }, + ); + } assert.deepEqual((await saved()).get("stored"), before); }); + it("should only load entries marked deleted: true as tombstones", async function() { + // Imports from before the marker was reserved could store deleted: false + const data = playerData("stored", 4, { epoch: 1 }); + const entries = [["stored", { ...data, deleted: false }]]; + await fs.writeFile(path.join(databaseDir, "player_sync.json"), JSON.stringify(entries)); + controller = await load(); + const response = await handler(msg.GetPlayerDataRequest)(new msg.GetPlayerDataRequest("stored")); + assert.deepEqual(response.playerData, data); + }); }); describe("DeletePlayerDataRequest", function() { diff --git a/plugins/player_sync/test/plugin.js b/plugins/player_sync/test/plugin.js index 4752f356..52f8d0cf 100644 --- a/plugins/player_sync/test/plugin.js +++ b/plugins/player_sync/test/plugin.js @@ -51,6 +51,14 @@ describe("player_sync plugin", function() { assert(!validate(request), `accepted ${generation}`); } }); + it("should reject uploads carrying the tombstone marker", function() { + const validate = lib.compile(msg.UploadRequest.jsonSchema); + assert(validate({ instanceId: 1, playerName: "player", playerData: playerData("player", 5) })); + for (const deleted of [false, true]) { + const data = playerData("player", 5, { deleted }); + assert(!validate({ instanceId: 1, playerName: "player", playerData: data }), `accepted ${deleted}`); + } + }); describe("SetPlayerDataRequest schema", function() { const validate = lib.compile(msg.SetPlayerDataRequest.jsonSchema); // Shaped like module/serialize.lua output @@ -116,6 +124,8 @@ describe("player_sync plugin", function() { for (const [field, value] of [ ["controller", "not-a-controller"], ["controller", "remote"], + ["deleted", false], + ["deleted", true], ["color", "red"], ["force", 1], ["ticks_to_respawn", -1],