From a8f3a3155c8c12f7742d5f258c7fb17e28254780 Mon Sep 17 00:00:00 2001 From: Andrew Quenehen Date: Mon, 3 Aug 2026 08:26:56 -0300 Subject: [PATCH] fix(nx): harden save inbox against slot clones Retire successful imports to *.sav.imported and record content hashes so re-pressing Import save (or the same bytes under a new name) cannot clone slots. Surface multi-import counts and the active game tab in the notice. Co-authored-by: Cursor --- .specs/STATE.md | 8 +- docs/launcher.md | 7 +- docs/switch-development.md | 5 +- docs/switch-install.md | 10 +- src/import/RomImporter.lua | 107 +++++++++++++++++---- tests/rom_importer_nx_saves_inbox_test.lua | 87 +++++++++++++++-- 6 files changed, 182 insertions(+), 42 deletions(-) diff --git a/.specs/STATE.md b/.specs/STATE.md index 612bb3d0..be96a8a6 100644 --- a/.specs/STATE.md +++ b/.specs/STATE.md @@ -91,12 +91,12 @@ - **Status**: active ### AD-012 -- **Decision**: On NX, raw Gen1 `.sav` import uses a writable inbox at `love.filesystem.getSaveDirectory()/imports/saves/` with **Import save** ensuring the dir and rescanning; export success surfaces `exports/` via an MTP path notice (no `openURL` / Open folder). Resilience guards RES-01..11 in `.specs/features/switch-save-sav-inbox/spec.md` apply (nested ensure, AppleDouble skip, retain inbox bytes, `isNX`-only branching, inbox isolation, non-silent notices). -- **Reason**: love-nx has no usable Horizon file picker (same scar as AD-003/AD-006); players need MTP/SD/FTP parity for continuing cart/PC saves and pulling slots off-console. -- **Trade-off**: Users must copy `.sav` into the shown inbox and pull exports manually; desktop/Android picker paths stay unchanged. +- **Decision**: On NX, raw Gen1 `.sav` import uses a writable inbox at `love.filesystem.getSaveDirectory()/imports/saves/` with **Import save** ensuring the dir and rescanning into the **active game tab**; export success surfaces `exports/` via an MTP path notice (no `openURL` / Open folder). Resilience guards RES-01..11 apply. After a successful import the live `.sav` is retired to `*.sav.imported` and its content hash is recorded in `imports/saves/.imported-sha1` so re-press / same-bytes-new-name cannot clone slots; failures leave the original file. +- **Reason**: love-nx has no usable Horizon file picker (same scar as AD-003/AD-006); players need MTP/SD/FTP parity for continuing cart/PC saves and pulling slots off-console. Unlimited re-import of a retained `.sav` was a slot-clone footgun. +- **Trade-off**: Users must copy `.sav` into the shown inbox and pull exports manually; desktop/Android picker paths stay unchanged; retired `.imported` files may accumulate until the player deletes them. - **Scope**: RomImporter SAVE FILES on Switch, Switch install/transfer/development/launcher docs, `tests/rom_importer_nx_saves_inbox_test.lua` - **Date**: 2026-08-03 -- **Status**: active +- **Status**: active (amended 2026-08-03 — retire + hash dedupe) ## Handoff diff --git a/docs/launcher.md b/docs/launcher.md index 8478cacc..ea754ef9 100644 --- a/docs/launcher.md +++ b/docs/launcher.md @@ -172,8 +172,11 @@ through `src/import/SaveFileIO.lua`, which sits on top of writes it (`SaveData.writeSlot`), and makes it active (`SaveData.setActiveSlot`). The meta stamp is re-stamped off `gen1_import` to the current numeric format so `SaveData.load`'s migration pass accepts the slot. On success the SAVE SLOT - panel is refreshed with the new slot selected. Inbox `.sav` files are - retained after success or failure. + panel is refreshed with the new slot selected. On **NX**, a successful inbox + import retires the file to `*.sav.imported` and records a content hash in + `imports/saves/.imported-sha1` so a second **Import save** (or the same bytes + under a new name) does not clone slots; failures leave the original `.sav`. + Imports always target the **active game tab** — use Red vs Blue accordingly. - **Export save** is live only when the active slot actually holds a save (checked against `listSlots`). `SaveFileIO.exportActiveSlot` loads the active slot, encodes it back with `SaveConvert.exportSav` (a slot never keeps diff --git a/docs/switch-development.md b/docs/switch-development.md index 2645e67c..28d46365 100644 --- a/docs/switch-development.md +++ b/docs/switch-development.md @@ -383,10 +383,11 @@ Raw Gen1 battery images use a **separate** MTP inbox (not mixed into ROM `import | Save-relative path | `imports/saves/` | | MTP destination | `1: SD Card//imports/saves/` (see launcher notice for the live `getSaveDirectory()` path) | | Candidates | non-hidden `*.sav` only | -| Rescan | SAVE FILES → **Import save** (imports each valid `.sav` via `SaveFileIO.importToSlot`; source files are retained on success and failure) | +| Rescan | SAVE FILES → **Import save** on the matching game tab (imports each *new* `.sav` via `SaveFileIO.importToSlot` into **that tab’s** slots) | +| After success | Retire to `*.sav.imported` + append content hash to `imports/saves/.imported-sha1` (re-press / same bytes under a new name → skip, no clone slots). Failures leave the original `.sav` | | Exports | **Export save** writes under `exports/`; NX shows an MTP path notice (no `openURL` / Open folder) | -Do **not** commit `.sav` bytes into git. Drop the file over MTP, press **Import save**, then play from the new slot. Pull exports from `exports/` the same way. +Do **not** commit `.sav` bytes into git. Drop the file over MTP, press **Import save** on the correct game tab, then play from the new slot. Pull exports from `exports/` the same way. **MTP tip:** the same AppleDouble `._*.sav` rule applies — see the mod inbox tip above. diff --git a/docs/switch-install.md b/docs/switch-install.md index 2dc64bf4..f0e6b4e1 100644 --- a/docs/switch-install.md +++ b/docs/switch-install.md @@ -71,9 +71,13 @@ SD / FTP — same transfer methods as ROMs: 1. Copy a Gen1 `.sav` (32 KB) into the save-dir **`imports/saves/`** path the launcher shows ([switch-transfer.md](switch-transfer.md)). -2. With the game’s ROM already imported, open **SAVE FILES** → **Import - save**. The launcher rescans the inbox and creates a new slot. -3. To pull a slot off the console, use **Export save**, then copy the file +2. With the game’s ROM already imported, open that game’s tab → **SAVE FILES** + → **Import save**. The launcher rescans the inbox into **this tab’s** + slots (Red vs Blue matter — use the matching game tab). +3. A successful import retires the file to `*.sav.imported` and records its + content hash so pressing **Import save** again does not clone slots. + Failed imports leave the original `.sav` in place. +4. To pull a slot off the console, use **Export save**, then copy the file from **`exports/`** in the same save directory via MTP / SD / FTP. Do not put `.sav` files into git. Prefer clean copies — some MTP clients diff --git a/src/import/RomImporter.lua b/src/import/RomImporter.lua index 5a14b3f2..b502e04c 100644 --- a/src/import/RomImporter.lua +++ b/src/import/RomImporter.lua @@ -343,6 +343,10 @@ end local IMPORTS_DIR = "imports" local MODS_INBOX_DIR = "imports/mods" local SAVES_INBOX_DIR = "imports/saves" +-- Ledger of successfully imported .sav content hashes (hidden → skipped by +-- listSavPaths). Prevents re-pressing Import save from cloning slots when the +-- same bytes are still in the inbox under a new name. +local SAVES_IMPORTED_HASHES = SAVES_INBOX_DIR .. "/.imported-sha1" local ROM_BYTES = 1024 * 1024 -- Strip only a validated sdmc:/ prefix for OpenMTP/DBI relative paths. @@ -503,6 +507,38 @@ function RomImporter:scanSavesInbox() return listSavPaths(SAVES_INBOX_DIR) end +local function loadImportedSavHashes() + local set = {} + local raw = love.filesystem.read(SAVES_IMPORTED_HASHES) + if type(raw) ~= "string" then return set end + for line in raw:gmatch("[^\r\n]+") do + local h = line:match("^(%x+)$") + if h then set[h] = true end + end + return set +end + +local function appendImportedSavHash(hash) + if type(hash) ~= "string" or hash == "" then return end + local prev = love.filesystem.read(SAVES_IMPORTED_HASHES) or "" + if prev:find(hash, 1, true) then return end + love.filesystem.write(SAVES_IMPORTED_HASHES, prev .. hash .. "\n") +end + +-- Keep bytes for the player (MTP recovery) but stop matching %.sav$ on rescan. +local function retireImportedSav(path) + if type(path) ~= "string" or path == "" then return false end + local data = love.filesystem.read(path) + if type(data) ~= "string" then return false end + local dest = path .. ".imported" + if love.filesystem.getInfo(dest) then + dest = path .. ".imported." .. tostring(os.time()) + end + if not love.filesystem.write(dest, data) then return false end + love.filesystem.remove(path) + return true +end + -- Rescan imports/mods/: install each .zip via _installMod / installZip. -- Never deletes inbox zips (success or failure). Empty inbox → MTP notice. function RomImporter:rescanModsAction() @@ -546,8 +582,11 @@ function RomImporter:rescanModsAction() end end --- Rescan imports/saves/: import each .sav via _importSave. Never deletes --- inbox .sav files (success or failure). Empty / AppleDouble-only → MTP notice. +-- Rescan imports/saves/: import each new .sav via _importSave. +-- Failure retains the original .sav. Success records a content hash and +-- retires the file to `*.sav.imported` so a second Import save cannot clone +-- slots (bytes stay in the inbox for MTP recovery). Already-hashed content +-- is skipped even under a new filename. Empty / AppleDouble-only → MTP notice. function RomImporter:rescanSavesAction(version) if self.workState == "working" then return end version = self:_resolveSaveVersion(version) @@ -557,32 +596,58 @@ function RomImporter:rescanSavesAction(version) self:_setNxSavesInboxNotice(version) return end - local anyOk = false - local lastOk = nil - local lastFail = nil - local failCount = 0 + local seenHashes = loadImportedSavHashes() + local okCount, failCount, skipCount = 0, 0, 0 + local lastOk, lastFail = nil, nil + local gameLabel = GameVersion.info(version).displayName for _, path in ipairs(candidates) do - self:_importSave(version, path) - local notice = self.saveNotice and self.saveNotice[version] - if notice and notice.ok then - anyOk = true - lastOk = notice + local data = love.filesystem.read(path) + local hash = (type(data) == "string" and data ~= "") and sha1(data) or nil + if hash and seenHashes[hash] then + skipCount = skipCount + 1 + -- Leftover live .sav after a prior success: retire without re-importing. + retireImportedSav(path) else - failCount = failCount + 1 - lastFail = notice + self:_importSave(version, path) + local notice = self.saveNotice and self.saveNotice[version] + if notice and notice.ok then + okCount = okCount + 1 + lastOk = notice + if hash then + seenHashes[hash] = true + appendImportedSavHash(hash) + end + retireImportedSav(path) + else + failCount = failCount + 1 + lastFail = notice + end end end - if anyOk and lastFail then - local okText = (lastOk and lastOk.text) or "Imported" - local failText = (lastFail and lastFail.text) or "unknown error" + if okCount > 0 then + local okText + if okCount == 1 and lastOk then + okText = Strings("%s (%s tab)", lastOk.text, gameLabel) + else + okText = Strings("Imported %d saves into %s. Active: %s.", + okCount, gameLabel, tostring(self.activeSlot[version])) + end + if failCount > 0 then + local failText = (lastFail and lastFail.text) or "unknown error" + okText = Strings("%s\n(%d failed: %s)", okText, failCount, failText) + end + if skipCount > 0 then + okText = Strings("%s\n(%d already imported, skipped)", okText, skipCount) + end + self.saveNotice[version] = { ok = true, text = okText } + elseif failCount > 0 then + self.saveNotice[version] = lastFail + elseif skipCount > 0 then self.saveNotice[version] = { ok = true, - text = Strings("%s\n(%d failed: %s)", okText, failCount, failText), + text = Strings("Already imported — %d file(s) skipped. Check SAVE SLOT.", + skipCount), } - elseif anyOk then - self.saveNotice[version] = lastOk - elseif lastFail then - self.saveNotice[version] = lastFail end end diff --git a/tests/rom_importer_nx_saves_inbox_test.lua b/tests/rom_importer_nx_saves_inbox_test.lua index 182e65e5..47f3dd4a 100644 --- a/tests/rom_importer_nx_saves_inbox_test.lua +++ b/tests/rom_importer_nx_saves_inbox_test.lua @@ -1,5 +1,5 @@ --- NX saves .sav inbox: ensure imports/saves/, MTP hint, AppleDouble/retain --- (NXSAV-01..10 + RES-01..08; RES-09/11 wired in later tasks). +-- NX saves .sav inbox: ensure imports/saves/, MTP hint, AppleDouble/retain/ +-- retire-on-success + hash dedupe (NXSAV + RES harden). package.path = "./?.lua;./?/init.lua;" .. package.path if not _G.love then _G.love = require("tests.love_stub") end @@ -9,16 +9,34 @@ local check = S.check local RomImporter = require("src.import.RomImporter") +love.data = love.data or {} love.system = love.system or {} love.filesystem = love.filesystem or {} local saved = { + hash = love.data.hash, + encode = love.data.encode, getOS = love.system.getOS, getSaveDirectory = love.filesystem.getSaveDirectory, createDirectory = love.filesystem.createDirectory, remove = love.filesystem.remove, } +-- Deterministic fake sha1 so content-hash dedupe is testable headless. +love.data.hash = function(_, data) + return data +end +love.data.encode = function(_, _, digest) + local s = type(digest) == "string" and digest or tostring(digest) + local hex = {} + for i = 1, #s do + hex[#hex + 1] = string.format("%02x", s:byte(i)) + end + local h = table.concat(hex) + if #h < 40 then h = h .. string.rep("0", 40 - #h) end + return h:sub(1, 40) +end + love.system.getOS = function() return "NX" end love.filesystem.getSaveDirectory = function() return "sdmc:/switch/gen1recomp/pokemon-love2d" @@ -50,6 +68,7 @@ local function clearSavesInbox() for _, name in ipairs(love.filesystem.getDirectoryItems("imports") or {}) do love.filesystem.remove("imports/" .. name) end + love.filesystem.remove("imports/saves/.imported-sha1") end local function freshImporter() @@ -186,7 +205,7 @@ eq(#importCalls, 0, "RES-02: AppleDouble-only does not import") check(ri.saveNotice.red ~= nil and ri.saveNotice.red.text:find("imports/saves/", 1, true), "RES-02: AppleDouble-only shows MTP notice") --- NXSAV-03 / RES-05: success → refresh; .sav retained (no remove) +-- NXSAV-03 / RES-05: success → refresh; bytes kept as *.sav.imported (not re-scanned) ri = freshImporter() importCalls = {} removed = {} @@ -198,11 +217,38 @@ eq(importCalls[1].source, "imports/saves/good.sav", "importToSlot receives inbox eq(importCalls[1].version, "red", "importToSlot uses panel version") check(ri._refreshed and ri._refreshed >= 1, "success refreshes slots") check(ri.saveNotice.red and ri.saveNotice.red.ok, "success sets ok notice") -check(not removed["imports/saves/good.sav"], "RES-05: success retains inbox .sav") -check(love.filesystem.read("imports/saves/good.sav") == "GOODSAV", - "RES-05: success leaves .sav bytes in inbox") +check(ri.saveNotice.red.text:find("Pokemon Red", 1, true) + or ri.saveNotice.red.text:find("Red", 1, true), + "success notice names the game tab") +check(love.filesystem.getInfo("imports/saves/good.sav") == nil, + "RES-05: success retires live .sav (no longer a candidate)") +check(love.filesystem.read("imports/saves/good.sav.imported") == "GOODSAV", + "RES-05: success keeps bytes under .sav.imported") +check(love.filesystem.getInfo("imports/saves/.imported-sha1") ~= nil, + "success records content hash ledger") --- NXSAV-04 / RES-05: failure → clear notice; .sav retained +-- Re-press Import save must not clone slots (hash ledger + retired file) +ri = freshImporter() +importCalls = {} +love.filesystem.write("imports/saves/again.sav", "SAMEBYTES") +importBehavior["imports/saves/again.sav"] = { ok = true, id = "slot-1" } +ri:rescanSavesAction("red") +eq(#importCalls, 1, "first import of again.sav") +-- Put the same bytes back under a new name (player re-copied / renamed) +love.filesystem.write("imports/saves/again-copy.sav", "SAMEBYTES") +importBehavior["imports/saves/again-copy.sav"] = { ok = true, id = "slot-clone" } +importCalls = {} +ri:rescanSavesAction("red") +eq(#importCalls, 0, "harden: same content hash is not imported again") +check(ri.saveNotice.red and ri.saveNotice.red.ok, + "harden: already-imported skip sets ok notice") +check(ri.saveNotice.red.text:find("Already imported", 1, true) + or ri.saveNotice.red.text:find("skipped", 1, true), + "harden: notice explains skip") +check(love.filesystem.getInfo("imports/saves/again-copy.sav") == nil, + "harden: leftover duplicate .sav is retired without importing") + +-- NXSAV-04 / RES-05: failure → clear notice; .sav retained as-is ri = freshImporter() importCalls = {} removed = {} @@ -217,7 +263,7 @@ check(not removed["imports/saves/bad.sav"], "RES-05: failure does not remove inb check(love.filesystem.read("imports/saves/bad.sav") == "BADSAV", "RES-05: failure leaves .sav in inbox") --- Mixed valid/invalid: attempt each; no .sav deleted +-- Mixed valid/invalid: attempt each; bad retained, good retired ri = freshImporter() importCalls = {} removed = {} @@ -227,14 +273,33 @@ importBehavior["imports/saves/a-bad.sav"] = { ok = false, err = "bad checksum" } importBehavior["imports/saves/b-good.sav"] = { ok = true, id = "slot-b" } ri:rescanSavesAction("red") eq(#importCalls, 2, "mixed inbox attempts each .sav") -check(not removed["imports/saves/a-bad.sav"], "mixed: bad .sav retained") -check(not removed["imports/saves/b-good.sav"], "mixed: good .sav retained") +check(love.filesystem.read("imports/saves/a-bad.sav") == "BAD", + "mixed: bad .sav retained") +check(love.filesystem.getInfo("imports/saves/b-good.sav") == nil, + "mixed: good .sav retired") +check(love.filesystem.read("imports/saves/b-good.sav.imported") == "GOOD", + "mixed: good bytes kept as .imported") check(ri.saveNotice.red and ri.saveNotice.red.ok, "mixed keeps overall success when one imports") check(ri.saveNotice.red.text:find("failed", 1, true), "mixed success notice still surfaces sibling failure") check(ri.saveNotice.red.text:find("bad checksum", 1, true), "mixed success notice includes the failure reason") +-- Multi-success notice names count + active slot (not only last ok line) +ri = freshImporter() +importCalls = {} +love.filesystem.write("imports/saves/one.sav", "ONE") +love.filesystem.write("imports/saves/two.sav", "TWO") +importBehavior["imports/saves/one.sav"] = { ok = true, id = "slot-one" } +importBehavior["imports/saves/two.sav"] = { ok = true, id = "slot-two" } +ri:rescanSavesAction("red") +eq(#importCalls, 2, "multi-success imports each distinct .sav") +check(ri.saveNotice.red.text:find("Imported 2 saves", 1, true), + "multi-success notice reports count") +check(ri.saveNotice.red.text:find("Active:", 1, true), + "multi-success notice reports active slot") +eq(ri.activeSlot.red, "slot-two", "multi-success leaves last import active") + -- RES-03: Mac MTP AppleDouble (._*.sav) must not be import candidates ri = freshImporter() importCalls = {} @@ -424,6 +489,8 @@ clearSavesInbox() love.filesystem.remove("imports/other.sav") package.loaded["src.import.SaveFileIO"] = nil package.loaded["src.core.HostShell"] = nil +love.data.hash = saved.hash +love.data.encode = saved.encode love.system.getOS = saved.getOS love.system.openURL = nil love.filesystem.getSaveDirectory = saved.getSaveDirectory