From 542856c83d1991c3541e6286117648e8964a4bd5 Mon Sep 17 00:00:00 2001 From: anxiousintrovert <82425472+anxiousintrovert@users.noreply.github.com> Date: Fri, 14 Aug 2026 14:22:46 -0500 Subject: [PATCH 1/3] Add manifest-driven required mod imports --- .gitignore | 5 + docs/modding.md | 41 +++ docs/rfcs/0010-required-mod-imports.md | 105 +++++++ .../jni/love/src/modules/system/System.cpp | 13 + .../src/jni/love/src/modules/system/System.h | 4 +- .../love/src/modules/system/wrap_System.cpp | 7 + mobile/ios/native/GRPickerBridge.swift | 6 +- mobile/ios/patch_love_src.py | 3 +- src/import/LauncherView.lua | 120 +++++++- src/import/RomImporter.lua | 210 +++++++++++++- src/mods/LauncherMods.lua | 72 ++++- src/mods/Loader.lua | 20 +- src/mods/Manifest.lua | 78 ++++- src/mods/RequiredImports.lua | 266 ++++++++++++++++++ tests/launcher_mods_install_zip_test.lua | 27 ++ tests/mod_required_imports_tests.lua | 193 +++++++++++++ tests/modkit_tests.lua | 4 + tests/rom_importer_android_mod_pick_test.lua | 56 ++++ tools/modkit.py | 8 + 19 files changed, 1224 insertions(+), 14 deletions(-) create mode 100644 docs/rfcs/0010-required-mod-imports.md create mode 100644 src/mods/RequiredImports.lua create mode 100644 tests/mod_required_imports_tests.lua diff --git a/.gitignore b/.gitignore index 69581d85..0f509ed2 100644 --- a/.gitignore +++ b/.gitignore @@ -79,3 +79,8 @@ mobile/ios/bundle_id.local # Local options / preferences /options.lua* + +# User-owned ROMs imported for individual mods. Manifests declare the +# destinations, but source checkouts and packaged mods never ship the files. +/mods/*/baseroms/ +/imports/baseroms/ diff --git a/docs/modding.md b/docs/modding.md index 8a1a287c..621430dc 100644 --- a/docs/modding.md +++ b/docs/modding.md @@ -44,6 +44,23 @@ Every mod contains a root `manifest.json` defining its metadata, supported games "optional_dependencies": [ "gen1_modern_ui" ], + "required_imports": [ + { + "id": "stadium2", + "name": "Pokemon Stadium 2 ROM", + "file": "stadium2.z64", + "format": "n64", + "md5": ["00000000000000000000000000000000"] + } + ], + "optional_imports": [ + { + "id": "bonus_source", + "name": "Optional bonus source", + "file": "bonus.bin", + "md5": "00000000000000000000000000000000" + } + ], "conflicts": [], "permissions": ["engine_internals"], "description": "A brief description of the mod.", @@ -67,6 +84,8 @@ Every mod contains a root `manifest.json` defining its metadata, supported games | `priority` | `integer` | Load priority order (lower numbers load earlier; dependencies always precede dependents regardless of priority). | | `dependencies` | `array` | Hard required dependencies. A mod will not load if a required dependency is missing or disabled for the active game. | | `optional_dependencies` | `array` | Soft dependencies. Guarantees that if the target mod is present and active, it loads *before* this mod without blocking load if absent. | +| `required_imports` | `array` | User-supplied files required by this mod. The launcher validates and copies each file into this mod's `baseroms/` directory; the mod does not load while one is missing. | +| `optional_imports` | `array` | User-supplied files that unlock optional mod functionality. They use the same validation and private-copy flow but never block the mod from loading. | | `conflicts` / `incompatible` | `array` | List of mod IDs that cannot run concurrently with this mod. | | `permissions` | `array` | Requested privileges (e.g. `["engine_internals"]`, `["network"]`, `["filesystem"]`). | | `github` | `string` | GitHub repository (`"owner/repo"`) used for update checks and dependency download links. | @@ -91,6 +110,28 @@ Dependencies in `dependencies` and `optional_dependencies` can be declared in se #### Version-Scoped Dependencies When a mod supports multiple games (`"games": ["gen1", "gen2"]`), a dependency can specify `"games": ["gen2"]` to indicate it is only required when booting Gen 2. When booting Gen 1, the engine will ignore the dependency, preventing unnecessary boot blocks on games that do not need it. +### Required user-supplied files + +`required_imports` and `optional_imports` keep copyrighted or otherwise user-owned source material +out of mod archives while giving every platform the same installation flow. +Each object requires a stable `id`, a display `name`, a destination `file` +(a filename, never a path), and one MD5 digest or an array of accepted MD5 +digests. `format` is either `"raw"` (the default) or `"n64"`. + +For `"n64"`, the launcher recognizes `.z64`, `.v64`, and `.n64` byte orders, +strips a recognized 512-byte copier header, converts the bytes to canonical +big-endian `.z64` order, and then checks MD5. The canonical bytes are written +to `mods//baseroms/`. Another installed mod with an overlapping +accepted MD5 automatically supplies a copy, so the player only selects a ROM +once. Mods read the result with their existing scoped `mod:read` API, for +example `mod:read("baseroms/stadium2.z64")`; no host path or new filesystem +permission is exposed. Missing `required_imports` block the mod before its +entry chunk runs; missing `optional_imports` remain visible in the same +launcher panel but do not block loading. + +MD5 here identifies a known dump; it is not used as a security or authenticity +guarantee. Mod archives must not include anything beneath `baseroms/`. + ## Mods and Gold (Gen 2) The mod API is one API across both generations, but Gold runs its own battle diff --git a/docs/rfcs/0010-required-mod-imports.md b/docs/rfcs/0010-required-mod-imports.md new file mode 100644 index 00000000..3196bee8 --- /dev/null +++ b/docs/rfcs/0010-required-mod-imports.md @@ -0,0 +1,105 @@ +# RFC 0010 — Manifest-declared user imports for mods + +## Status + +Proposed. Engine: `Manifest.lua`, `RequiredImports.lua`, `Loader.lua`, +`LauncherMods.lua`. Launcher: `RomImporter.lua`, `LauncherView.lua`. Native +picker bridges: Android and iOS. + +## Motivation + +Some mods derive presentation data from another cartridge the player owns. +Shipping those bytes in a mod is not acceptable, while asking each mod to +escape the sandbox, implement native pickers, and maintain platform-specific +paths defeats the sandbox's purpose. Pokemon Stadium and Pokemon Stadium 2 are +the first concrete consumers, but the ownership/import problem is generic. + +## The decision it extends + +This extends the manifest as the engine-owned declaration of mod dependencies +and preserves the sandbox rule in `src/mods/Sandbox.lua`: mods do not receive +raw host filesystem access. It also extends the launcher's established ROM, +save, and mod archive picker/inbox flows instead of introducing a second host +integration. + +## Exact manifest delta + +Additive `required_imports` and `optional_imports` arrays are accepted: + +```json +{ + "required_imports": [{ + "id": "stadium2", + "name": "Pokemon Stadium 2 ROM", + "file": "stadium2.z64", + "format": "n64", + "md5": ["00000000000000000000000000000000"] + }] +} +``` + +Both arrays use the same object schema. A missing `required_imports` entry +blocks the mod; a missing `optional_imports` entry only leaves that bonus +functionality unavailable. + +- `id` is unique within the manifest and uses the mod-id vocabulary. +- `name` is launcher-facing text. +- `file` is one safe filename below the mod's `baseroms/` directory. +- `md5` is one 32-digit hexadecimal digest or a non-empty array of accepted + digests. MD5 is a known-dump identity convention, not a trust primitive. +- `format` is `raw` by default. `n64` recognizes a canonical big-endian dump, + pair-byte-swapped and little-endian-word dumps, with or without a recognized + 512-byte copier header. Validation and stored output use canonical big-endian + bytes. + +The launcher copies a validated file to +`mods//baseroms/`. Missing required imports make the launcher +row need attention and make the loader refuse that enabled mod before its entry +chunk runs. Missing optional imports remain selectable without changing load +status. A matching validated import owned by another installed mod is +copied automatically. Replace and remove remain explicit per-mod actions. + +Desktop and UWP use their existing native/host picker routes. Android and iOS +add a `required_import` picker kind which stages +`picked_required_import.bin`. NX scans the engine-owned +`imports/baseroms/` MTP inbox. All writes continue through `CacheFs`, preserving +portable-mode placement. + +`modkit validate` and `modkit pack` report MK307 for every file beneath a +source mod's `baseroms/` directory, so the packaging path cannot accidentally +distribute a file the launcher placed there. The installer also rejects a mod +archive containing `baseroms/` files, covering packages built without modkit. + +## Mod-facing API and sandbox statement + +There is no new runtime API and no new permission. A mod reads its own copied +file through the existing `mod:read("baseroms/")` capability. It never +learns the selected host path, cannot browse another mod's tree, and receives +no raw `io`, `love.filesystem`, or platform-picker access. + +## Migration and compatibility + +Existing manifests omit both import arrays and behave exactly as before. The +fields are additive for both manifest API levels. Mods currently maintaining +their own cross-platform picker can declare the source file and remove that +host integration; their processing code changes only to read the declared +`baseroms/` path. + +## Parity tests + +- Empty/absent `required_imports` leaves existing manifests and loader behavior + unchanged. +- Manifest validation refuses path traversal, duplicate ids/files, malformed + MD5 values, and unknown normalization formats. +- N64 byte orders and the recognized copier-header form produce identical + canonical bytes before MD5 validation. +- A mismatched selection is never written. +- An enabled mod with a missing declared file never executes its entry chunk. +- A matching import in another installed mod is copied into the requesting + mod's own `baseroms/` directory. +- The packaging gate refuses every `baseroms/` file. +- The launcher refuses an archive that contains `baseroms/` files. + +## Deprecation etiquette + +Nothing deprecated or removed. diff --git a/mobile/android/love/src/jni/love/src/modules/system/System.cpp b/mobile/android/love/src/jni/love/src/modules/system/System.cpp index b2b4ac5b..e5f84b24 100644 --- a/mobile/android/love/src/jni/love/src/modules/system/System.cpp +++ b/mobile/android/love/src/jni/love/src/modules/system/System.cpp @@ -192,8 +192,12 @@ bool System::pickFile(const char *kind) const dest = "picked_mod.zip"; else if (strcmp(kind, "sav") == 0 || strcmp(kind, "save") == 0) dest = "picked_save.sav"; + else if (strcmp(kind, "required_import") == 0) + dest = "picked_required_import.bin"; else if (strcmp(kind, "rom") == 0) dest = "picked_rom.gb"; + else + return false; } return love::android::showFilePicker(dest); #else @@ -202,6 +206,15 @@ bool System::pickFile(const char *kind) const #endif } +const char *System::pickFileKinds() const +{ +#ifdef LOVE_ANDROID + return "rom,mod,sav,required_import"; +#else + return ""; +#endif +} + bool System::createFile(const char *suggestedName) const { #ifdef LOVE_ANDROID diff --git a/mobile/android/love/src/jni/love/src/modules/system/System.h b/mobile/android/love/src/jni/love/src/modules/system/System.h index ccf3538d..7ab0484a 100644 --- a/mobile/android/love/src/jni/love/src/modules/system/System.h +++ b/mobile/android/love/src/jni/love/src/modules/system/System.h @@ -112,10 +112,12 @@ public: * love::android::showFilePicker and src/import/RomImporter.lua. * * @param kind Optional pick kind: nullptr/"rom" -> picked_rom.gb, - * "mod" -> picked_mod.zip, "sav"/"save" -> picked_save.sav. + * "mod" -> picked_mod.zip, "sav"/"save" -> picked_save.sav, + * "required_import" -> picked_required_import.bin. * @return Whether the picker was shown. **/ virtual bool pickFile(const char *kind = nullptr) const; + virtual const char *pickFileKinds() const; /** * Shows the platform's native "create / save a file" UI (Android SAF diff --git a/mobile/android/love/src/jni/love/src/modules/system/wrap_System.cpp b/mobile/android/love/src/jni/love/src/modules/system/wrap_System.cpp index fc8c2240..9dfa18ea 100644 --- a/mobile/android/love/src/jni/love/src/modules/system/wrap_System.cpp +++ b/mobile/android/love/src/jni/love/src/modules/system/wrap_System.cpp @@ -102,6 +102,12 @@ int w_pickFile(lua_State *L) return 1; } +int w_pickFileKinds(lua_State *L) +{ + luax_pushstring(L, instance()->pickFileKinds()); + return 1; +} + int w_createFile(lua_State *L) { const char *suggested = luaL_optstring(L, 1, nullptr); @@ -222,6 +228,7 @@ static const luaL_Reg functions[] = { "openURL", w_openURL }, { "vibrate", w_vibrate }, { "pickFile", w_pickFile }, + { "pickFileKinds", w_pickFileKinds }, { "createFile", w_createFile }, { "syncHealthSteps", w_syncHealthSteps }, { "restartApp", w_restartApp }, diff --git a/mobile/ios/native/GRPickerBridge.swift b/mobile/ios/native/GRPickerBridge.swift index c571461b..b51925f6 100644 --- a/mobile/ios/native/GRPickerBridge.swift +++ b/mobile/ios/native/GRPickerBridge.swift @@ -5,7 +5,7 @@ // without updating that patch (see mobile/ios/patch_love_src.py). // // Contract (mirrors love-android's GameActivity.showFilePicker): -// love.system.pickFile("rom"|"mod"|"sav") -> copies the user's pick into +// love.system.pickFile("rom"|"mod"|"sav"|"required_import") -> copies the user's pick into // the LÖVE save directory as picked_rom.gb / picked_mod.zip / // picked_save.sav; RomImporter's pending-file scan consumes it. // love.system.createFile(name) -> exports save dir's pending_export.sav @@ -83,6 +83,8 @@ public final class GRPickerBridge: NSObject { types = [.zip] case "sav": destName = "picked_save.sav" + case "required_import": + destName = "picked_required_import.bin" // A Nintendo 64 cartridge, for mods that build assets out of one -- // the voxel mod's Pokemon Stadium battle models are the caller this // was added for. Its own filename on purpose: an N64 ROM landing on @@ -135,7 +137,7 @@ public final class GRPickerBridge: NSObject { // Kept beside the switch it describes, because the two drifting apart is // the only way this can lie. @objc public static func supportedPickerKinds() -> NSString { - return "rom,mod,sav,stadium" as NSString + return "rom,mod,sav,stadium,required_import" as NSString } @objc(presentExportWithName:saveDir:) diff --git a/mobile/ios/patch_love_src.py b/mobile/ios/patch_love_src.py index e9a90a43..0ddbdc5b 100644 --- a/mobile/ios/patch_love_src.py +++ b/mobile/ios/patch_love_src.py @@ -89,7 +89,8 @@ int w_pickFile(lua_State *L) return gr_callBridge(L, "GRPickerBridge", "presentPickerWithKind:saveDir:", kind); } -// love.system.pickFileKinds() -> "rom,mod,sav,stadium", or nil off iOS. +// love.system.pickFileKinds() -> the comma-separated kinds supported by the +// Swift bridge (including required_import), or nil off iOS. // // So a caller can ask what this build's picker understands BEFORE opening it. // An unknown kind is refused (GRPickerBridge), and a refusal looks exactly diff --git a/src/import/LauncherView.lua b/src/import/LauncherView.lua index 05f66799..eb4a49d3 100644 --- a/src/import/LauncherView.lua +++ b/src/import/LauncherView.lua @@ -638,6 +638,7 @@ end local function modStatusColor(status) if status == "ok" then return Strings("Ready"), PAL.green end + if status == "needs_import" then return Strings("Import required"), PAL.yellow end if status == "conflict" then return Strings("Conflict"), PAL.red end -- not a fault: the mod is intact, this is simply not a game it is for -- (src/mods/ModTargets.lua) @@ -2767,11 +2768,14 @@ local function buildModActionsModal(imp, m) local hasGit = mod.github and mod.github ~= "" local depSpecs = mod.dependencySpecs or (mod.manifest and mod.manifest.dependencySpecs) local hasDeps = depSpecs and #depSpecs > 0 + local imports = mod.imports or mod.requiredImports + local hasImports = imports and #imports > 0 local info = hasGit and imp:_modUpdateInfo(mod.id) local pad = math.floor(18 * m.s) local w = math.floor(440 * m.s) local gap = math.floor(8 * m.s) - local nBtns = (hasGit and 2 or 0) + (hasDeps and 1 or 0) + 2 + local nBtns = (hasGit and 2 or 0) + (hasDeps and 1 or 0) + + (hasImports and 1 or 0) + 2 local h = pad + Kit.textHeight("button") + math.floor(4 * m.s) + Kit.textHeight("small") + math.floor(12 * m.s) + nBtns * (m.btnH + gap) - gap + pad @@ -2821,6 +2825,19 @@ local function buildModActionsModal(imp, m) end }) cy = cy + m.btnH + gap end + if hasImports then + local missing = tonumber(mod.missingRequiredImports) or 0 + local label = missing > 0 + and Strings("Imported files (%d required)", missing) + or Strings("Imported files") + btn(imp, px + pad, cy, pw - 2 * pad, m.btnH, "modact-imports", + label, { kind = missing > 0 and "warn" or "accent", font = "small", + action = function() + imp._modImports = id + imp._modActions = nil + end }) + cy = cy + m.btnH + gap + end local armed = deleteArmed(imp, "mod", id, nil) btn(imp, px + pad, cy, pw - 2 * pad, m.btnH, "modact-del", DELETE_LABEL(armed), { @@ -2837,6 +2854,103 @@ local function buildModActionsModal(imp, m) action = function() imp._modActions = nil end }) end +-- Imported files declared by one installed mod. The engine picks, validates, +-- canonicalizes and copies; this surface never exposes a host path to mod code. +local function buildRequiredImportsModal(imp, m) + local mod + for _, candidate in ipairs(imp.mods or {}) do + if candidate.id == imp._modImports then mod = candidate break end + end + if not mod then imp._modImports = nil return end + local imports = mod.imports or mod.requiredImports or {} + local pad, gap = math.floor(18 * m.s), math.floor(8 * m.s) + local w = math.floor(540 * m.s) + local notice = imp.requiredImportNotice + if not notice or notice.modId ~= mod.id then notice = nil end + local noticeText + if notice then + local importName = notice.importId + for _, row in ipairs(imports) do + if row.id == notice.importId then importName = row.name break end + end + noticeText = Strings("%s rejected: %s", importName, notice.text) + end + local noticeW = w - 2 * pad + local noticeH = noticeText and Kit.wrapHeight("small", noticeText, noticeW, 2) or 0 + local rowH = math.max(math.floor(56 * m.s), m.btnH) + local perPage = math.min(4, math.max(1, #imports)) + local pagerH = #imports > perPage and math.max(Kit.tapMin(), math.floor(30 * m.s)) or 0 + local h = pad + Kit.textHeight("button") + math.floor(4 * m.s) + + Kit.textHeight("small") + math.floor(12 * m.s) + + noticeH + (noticeH > 0 and gap or 0) + + perPage * rowH + math.max(0, perPage - 1) * gap + + (pagerH > 0 and (gap + pagerH) or 0) + gap + m.btnH + pad + local px, py, pw = modalPanel(m, w, h) + local cy = py + pad + Kit.text("button", Kit.ellipsize("button", mod.name, pw - 2 * pad), + px + pad, cy, PAL.heading) + cy = cy + Kit.textHeight("button") + math.floor(4 * m.s) + Kit.text("small", Strings("User-supplied files are validated by MD5 and copied into this mod only."), + px + pad, cy, PAL.muted) + cy = cy + Kit.textHeight("small") + math.floor(12 * m.s) + if noticeText then + cy = cy + Kit.textWrapped("small", noticeText, px + pad, cy, + pw - 2 * pad, PAL.red, 2) + gap + end + + local pageKey = "required-imports-" .. mod.id + local cur = page(imp, pageKey) + local first, last, bounded = Kit.pageBounds(cur, #imports, perPage) + setPage(imp, pageKey, bounded) + for i = first, last do + local row = imports[i] + local importId = row.id + Kit.card(px + pad, cy, pw - 2 * pad, rowH, row.present and "muted" or false) + local innerX = px + pad + math.floor(12 * m.s) + local actionW = math.floor(108 * m.s) + local removeW = row.present and math.floor(86 * m.s) or 0 + local actionX = px + pw - pad - math.floor(10 * m.s) - actionW + if removeW > 0 then actionX = actionX - removeW - math.floor(6 * m.s) end + local textW = actionX - innerX - math.floor(8 * m.s) + Kit.text("small", Kit.ellipsize("small", row.name, textW), innerX, + cy + math.floor(8 * m.s), PAL.heading) + local state = row.present and Strings("Ready - %s", row.file) + or (row.error and Strings("Invalid file - choose again") + or (row.required and Strings("Required - %s", row.file) + or Strings("Optional - %s", row.file))) + Kit.text("micro", Kit.ellipsize("micro", state, textW), innerX, + cy + math.floor(8 * m.s) + Kit.textHeight("small") + math.floor(3 * m.s), + row.present and PAL.green or (row.required and PAL.yellow or PAL.muted)) + btn(imp, actionX, cy + (rowH - m.btnH) / 2, actionW, m.btnH, + "req-pick-" .. mod.id .. "-" .. importId, + row.present and Strings("Replace") or Strings("Choose file"), { + kind = row.present and "ghost" or "accent", font = "small", + action = function() imp:chooseRequiredImport(mod.id, importId) end }) + if row.present then + local deleteId = mod.id .. ":" .. importId + local armed = deleteArmed(imp, "required-import", deleteId, nil) + btn(imp, actionX + actionW + math.floor(6 * m.s), + cy + (rowH - m.btnH) / 2, removeW, m.btnH, + "req-remove-" .. mod.id .. "-" .. row.id, DELETE_LABEL(armed), { + kind = "danger", font = "small", keepArm = true, + action = function() + imp:pressDelete("required-import", deleteId, nil, function() + imp:_removeRequiredImport(mod.id, importId) + end) + end }) + end + cy = cy + rowH + gap + end + if pagerH > 0 then + local newPage = Kit.pager(px + pad, cy, pw - 2 * pad, bounded, + #imports, perPage, pageKey) + setPage(imp, pageKey, newPage) + cy = cy + pagerH + gap + end + btn(imp, px + pad, cy, pw - 2 * pad, m.btnH, "req-close", Strings("Close"), { + font = "small", action = function() imp._modImports = nil end }) +end + -- Per-mod popup for FIND MODS: the row is a plain click, and Install / -- Details / Source live here instead of crowding every row. local function buildFindEntryModal(imp, m) @@ -3377,7 +3491,8 @@ local function modalUp(imp) or imp._indexPrompt or imp._modConfirm or imp._modReleaseNotes or imp._appPatchNotes or imp._findDetails or imp._modVersions or imp._modDepResolver or imp._sortPopup - or imp._filterPopup or imp._modScopePopup or imp._indexManage or imp._modActions + or imp._filterPopup or imp._modScopePopup or imp._indexManage + or imp._modActions or imp._modImports or imp._modHeaderActionsPopup or imp._profilesPopup or imp._singleProfileActions or imp._profileSavePrompt or imp._profileRenamePrompt or imp._findEntry or imp._gameManage) ~= nil end @@ -3509,6 +3624,7 @@ local function buildModals(imp, m) end if imp._modVersions then buildVersionsModal(imp, m) return true end if imp._modDepResolver then buildDepResolverModal(imp, m) return true end + if imp._modImports then buildRequiredImportsModal(imp, m) return true end -- The lighter popups come after the deep ones on purpose: opening -- Versions or Details from inside an actions popup draws the deeper modal -- while the popup's own state stays set, so closing the deep one drops diff --git a/src/import/RomImporter.lua b/src/import/RomImporter.lua index 4666f226..3336ff5d 100644 --- a/src/import/RomImporter.lua +++ b/src/import/RomImporter.lua @@ -981,6 +981,25 @@ local function findPendingSav(preferAny, skip) return nil end +local function pickerHasKind(kind) + local fn = love.system.pickFileKinds + if type(fn) ~= "function" then return false end + local ok, kinds = pcall(fn) + if not ok or type(kinds) ~= "string" then return false end + for token in kinds:gmatch("[^,%s]+") do + if token == kind then return true end + end + return false +end + +local function findPendingRequiredImport() + local names = { "picked_required_import.bin", "picked_stadium.z64" } + for _, name in ipairs(names) do + if love.filesystem.getInfo(name, "file") then return name end + end + return nil +end + -- Retire an Android pick once it has been through the installer / importer, -- whether or not it worked: a pick left on disk wins the scans above forever, -- so the next tap re-runs the same failing file and the picker never reopens @@ -1112,6 +1131,39 @@ local function chooseSav() return nil end +-- Generic user-supplied dependency picker. Validation is manifest-driven, +-- so the dialog intentionally permits every file extension; a wrong choice +-- cannot reach the mod because its canonical MD5 must match first. +local function chooseRequiredFile(label) + local prompt = shellSafe("Choose " .. tostring(label or "required file")) + local platform = love.system.getOS() + if platform == "OS X" then + return commandOutput( + ([[osascript -e 'POSIX path of (choose file with prompt "%s")' 2>/dev/null]]) + :format(prompt)) + elseif platform == "Windows" then + local script = table.concat({ + "Add-Type -AssemblyName System.Windows.Forms;", + "$d=New-Object System.Windows.Forms.OpenFileDialog;", + "$d.Title='" .. prompt .. "';", + "$d.Filter='All files (*.*)|*.*';", + "if($d.ShowDialog() -eq 'OK'){", + "$t=Join-Path $env:TEMP 'pokeport_required_import.bin';", + "Copy-Item -LiteralPath $d.FileName -Destination $t -Force;", + "[Console]::OutputEncoding=[Text.Encoding]::UTF8;", + "[Console]::Write($t)}", + }) + return commandOutput( + 'powershell -NoProfile -STA -Command "' .. script .. '"') + elseif platform == "Linux" then + local path = commandOutput( + ([[zenity --file-selection --title="%s" 2>/dev/null]]):format(prompt)) + if path then return path end + return commandOutput([[kdialog --getopenfilename "$HOME" 2>/dev/null]]) + end + return nil +end + -- The self-updater only surfaces on the real distributed build: a fused, -- interactive launcher with no scripted-run override. A dev / source checkout -- (unfused, where Boot.run already no-ops) or an autopilot / driver / @@ -1230,7 +1282,9 @@ function RomImporter.new(onComplete, opts) -- (refreshed lazily on first draw and after any toggle/install/delete); -- modScroll is the current paged list's inner scroll offset (px, clamped -- in draw); modNotice is the last install/delete result { ok, text }. - mods = nil, modScroll = 0, modNotice = nil, + -- requiredImportNotice stays inside the imported-files modal so validation + -- failures are visible beside the file picker that caused them. + mods = nil, modScroll = 0, modNotice = nil, requiredImportNotice = nil, -- Which game the MODS panel is answering for (a GameVersion id, nil = -- every game). Rows resolve their enable-state and their "runs here" -- verdict against it (src/mods/ModTargets.lua). @@ -1420,7 +1474,13 @@ function RomImporter:focus(f) local text = "Could not read the picked file. Reopen the picker and choose " .. "it with the Files (Documents) app, or copy it into: " .. love.filesystem.getSaveDirectory() - if pickError:find("picked_mod", 1, true) then + if pickError:find("picked_required_import", 1, true) + or pickError:find("picked_stadium", 1, true) then + self.modNotice = { ok = false, text = text } + self.pickerPendingKind = nil + self.pickerPendingModId = nil + self.pickerPendingImportId = nil + elseif pickError:find("picked_mod", 1, true) then self.modNotice = { ok = false, text = text } elseif pickError:find("picked_save", 1, true) then local version = self.androidPendingVersion or self:_savedropTarget() @@ -1431,6 +1491,20 @@ function RomImporter:focus(f) end return end + local requiredName = findPendingRequiredImport() + if requiredName then + local modId, importId = self.pickerPendingModId, self.pickerPendingImportId + self.pickerPendingKind = nil + self.pickerPendingModId, self.pickerPendingImportId = nil, nil + local imported = modId and importId + and self:_importRequiredSource(modId, importId, requiredName) + consumePick(self, requiredName, requiredName, imported) + if not modId or not importId then + self.modNotice = { ok = false, + text = "A picked dependency file had no pending mod request and was discarded." } + end + return + end local modName = findPendingMod(false, self.pickSkip) if modName then self:_installMod(modName) @@ -1738,6 +1812,123 @@ function RomImporter:chooseMod() if path then self:_installMod(path) end end +local function requiredManifest(self, modId) + for _, row in ipairs(self.mods or {}) do + if row.id == modId then return row.manifest, row end + end + return nil +end + +local function requiredImportNotice(self, modId, importId, text) + self.requiredImportNotice = { + modId = modId, + importId = importId, + text = tostring(text), + } +end + +function RomImporter:_importRequiredData(modId, importId, data) + local manifest = requiredManifest(self, modId) + if not manifest then + self.modNotice = { ok = false, text = "Required import failed: mod not found." } + return nil + end + local ok, result = require("src.mods.RequiredImports") + .importData(manifest, importId, data) + if ok then + self.requiredImportNotice = nil + self.modNotice = { ok = true, text = "Imported " .. tostring(importId) + .. " for " .. tostring(manifest.name or manifest.id) .. "." } + self:_refreshMods() + return true + end + -- Keep validation feedback on the imported-files page. A general Mods-page + -- notice is hidden by this modal and made MD5 failures especially easy to miss. + requiredImportNotice(self, modId, importId, result) + self.modNotice = nil + return nil +end + +function RomImporter:_importRequiredSource(modId, importId, source) + local data = love.filesystem.read(source) + if not data then data = readExternalPath(source) end + if not data then + requiredImportNotice(self, modId, importId, "Could not read the selected file.") + self.modNotice = nil + return nil + end + return self:_importRequiredData(modId, importId, data) +end + +function RomImporter:_removeRequiredImport(modId, importId) + local manifest = requiredManifest(self, modId) + if not manifest then return end + local ok, err = require("src.mods.RequiredImports").remove(manifest, importId) + if ok then + self.requiredImportNotice = nil + self.modNotice = { ok = true, text = "Deleted " .. tostring(importId) .. "." } + self:_refreshMods() + else + requiredImportNotice(self, modId, importId, err) + self.modNotice = nil + end +end + +-- Select and validate one manifest-declared file. NX has no host picker, so +-- its equivalent is an engine-owned imports/baseroms inbox that can be filled +-- over MTP; every other native/mobile picker lands on the same validation path. +function RomImporter:chooseRequiredImport(modId, importId) + if self.workState == "working" then return end + local manifest = requiredManifest(self, modId) + if not manifest then return end + local spec + for _, candidate in ipairs(require("src.mods.RequiredImports").specs(manifest)) do + if candidate.id == importId then spec = candidate break end + end + if not spec then return end + + if self.isNX then + local inbox = "imports/baseroms" + love.filesystem.createDirectory(inbox) + for _, name in ipairs(love.filesystem.getDirectoryItems(inbox) or {}) do + if name:sub(1, 1) ~= "." then + local path = inbox .. "/" .. name + local data = love.filesystem.read(path) + if data and self:_importRequiredData(modId, importId, data) then return end + end + end + requiredImportNotice(self, modId, importId, + "No matching file in imports/baseroms/. Copy it there over MTP, then try again.") + self.modNotice = nil + return + end + if self.nativePicker then + if self.mobileFileBridge and not pickerHasKind("required_import") then + requiredImportNotice(self, modId, importId, + "This app build cannot pick required mod files yet. Update the app and try again.") + self.modNotice = nil + return + end + self.pickerPendingKind = "required_import" + self.pickerPendingModId = modId + self.pickerPendingImportId = importId + if not pickFile("required_import") then + self.pickerPendingKind = nil + self.pickerPendingModId = nil + self.pickerPendingImportId = nil + requiredImportNotice(self, modId, importId, "Could not open the file picker.") + self.modNotice = nil + elseif self.android then + self.pickPending = true + self.pickTimer = 0 + end + return + end + + local path = chooseRequiredFile(spec.name) + if path then self:_importRequiredSource(modId, importId, path) end +end + -- Which game a dropped .sav imports into: a .sav has no version signature of -- its own, so it lands on the active game tab. When a non-game tab (mods) is -- showing, default to red -- the always-present first game -- rather than @@ -2045,7 +2236,8 @@ function RomImporter:_pollPickedFiles(dt) if not found then for _, name in ipairs(love.filesystem.getDirectoryItems("")) do local n = name:lower() - if n:match("%.gbc?$") or n == "picked_mod.zip" or n == "picked_save.sav" then + if n:match("%.gbc?$") or n == "picked_mod.zip" or n == "picked_save.sav" + or n == "picked_required_import.bin" or n == "picked_stadium.z64" then found = true break end @@ -2174,7 +2366,12 @@ function RomImporter:update(dt) local version = self.pickerPendingVersion self.pickerPendingKind = nil self.pickerPendingVersion = nil - if kind == "mod" then + if kind == "required_import" then + local modId, importId = self.pickerPendingModId, self.pickerPendingImportId + self.pickerPendingModId, self.pickerPendingImportId = nil, nil + if modId and importId then self:_importRequiredSource(modId, importId, path) end + if Platform.isUWP() then os.remove(path) end + elseif kind == "mod" then self:_installMod(path) if Platform.isUWP() and self.modNotice and self.modNotice.ok then os.remove(path) @@ -2196,7 +2393,10 @@ function RomImporter:update(dt) local version = self.pickerPendingVersion or self:_savedropTarget() self.pickerPendingKind = nil self.pickerPendingVersion = nil - if kind == "mod" then + if kind == "required_import" then + self.modNotice = { ok = false, text = errorText } + self.pickerPendingModId, self.pickerPendingImportId = nil, nil + elseif kind == "mod" then self.modNotice = { ok = false, text = errorText } elseif kind == "sav" then self.saveNotice[version] = { ok = false, text = errorText } diff --git a/src/mods/LauncherMods.lua b/src/mods/LauncherMods.lua index 4f7f2ba8..6679d977 100644 --- a/src/mods/LauncherMods.lua +++ b/src/mods/LauncherMods.lua @@ -38,6 +38,7 @@ local Version = require("src.core.Version") local SaveData = require("src.core.SaveData") local GameVersion = require("src.core.GameVersion") local CacheFs = require("src.import.CacheFs") +local RequiredImports = require("src.mods.RequiredImports") local LauncherMods = {} @@ -459,13 +460,33 @@ function LauncherMods.list(version) local ok, result = pcall(function() local options = SaveData.loadOptions() local manifests = discover() + -- A validated copy owned by another installed mod can satisfy the same + -- declared MD5 without asking the player to select the ROM twice. + local _, importState = RequiredImports.reconcile(manifests) -- The first build containing game-specific switches turns the old shared -- state into one explicit answer per installed mod and game. Saving here -- means users who only visit the launcher still receive the migration. if SaveData.migrateModEnablement(options, manifests) then SaveData.saveOptions(options) end - return LauncherMods.deriveList(manifests, options, version) + local rows = LauncherMods.deriveList(manifests, options, version) + for _, row in ipairs(rows) do + local state = importState[row.id] + or { rows = {}, missing = 0, missingOptional = 0 } + local imports, missing = state.rows, state.missing + row.requiredImports, row.missingRequiredImports = imports, missing + row.imports = imports + row.missingOptionalImports = state.missingOptional or 0 + if missing > 0 and row.status == "ok" then + row.status = "needs_import" + local first + for _, import in ipairs(imports) do + if not import.present then first = import break end + end + row.statusDetail = "Needs import: " .. (first and first.name or "required file") + end + end + return rows end) if not ok then -- a single bad options/mod file must not blank the launcher @@ -677,6 +698,26 @@ local function copyTree(src, dst) return true end +-- User-supplied baseroms are install state, not package content. Snapshot +-- them before replacing a mod tree so an update cannot make the player select +-- the same cartridge again (or destroy their only reusable copy if the new +-- archive later fails to copy). +local function snapshotTree(path, into, relative) + local fs = love.filesystem + into, relative = into or {}, relative or "" + local info = fs.getInfo(path) + if not info then return into end + if info.type == "directory" then + for _, name in ipairs(fs.getDirectoryItems(path) or {}) do + local rel = relative == "" and name or (relative .. "/" .. name) + snapshotTree(path .. "/" .. name, into, rel) + end + elseif relative ~= "" and into[relative] == nil then + into[relative] = fs.read(path) + end + return into +end + -- Delete an installed mod subtree. Enumeration stays on love.filesystem (the -- portable game folder is on its read path), but the deletes go through -- CacheFs so a portable install's real files actually go away instead of @@ -919,6 +960,12 @@ function LauncherMods._installZipInner(source, opts) return nil, ("zip is for '%s', expected '%s'") :format(manifest.id, opts.expectId) end + local packagedBaseroms = root .. "/baseroms" + if fs.getInfo(packagedBaseroms, "directory") + and #(fs.getDirectoryItems(packagedBaseroms) or {}) > 0 then + cleanup() + return nil, "mod archives must not include user-supplied baseroms/ files" + end local dest = "mods/" .. manifest.id local existing, installedSomewhere = sameIdTrees(fs, manifest.id) @@ -926,7 +973,11 @@ function LauncherMods._installZipInner(source, opts) cleanup() return nil, "a mod named '" .. manifest.id .. "' is already installed" end + local preservedBaseroms = {} if #existing > 0 then + for _, path in ipairs(existing) do + snapshotTree(path .. "/baseroms", preservedBaseroms) + end -- drop every old tree before copy -- mods/ and any same-id folder -- under another name, or the survivor keeps winning discover()'s -- first-id-wins race after the "successful" update (#801). A tree with @@ -950,6 +1001,25 @@ function LauncherMods._installZipInner(source, opts) CacheFs.prefix = "" local copied, copyErr = copyTree(root, dest) if not copied then removeTree(dest) end + local preserveErr + for rel, bytes in pairs(preservedBaseroms) do + if bytes ~= nil then + local restored, restoreErr = CacheFs.write(dest .. "/baseroms/" .. rel, bytes) + if not restored and not preserveErr then + preserveErr = "could not preserve baseroms/" .. rel .. ": " + .. tostring(restoreErr) + end + end + end + if preserveErr then + -- Do not report a successful update that discarded user-owned input. Keep + -- a best-effort baseroms-only tree for the next retry instead. + removeTree(dest) + for rel, bytes in pairs(preservedBaseroms) do + if bytes ~= nil then CacheFs.write(dest .. "/baseroms/" .. rel, bytes) end + end + copied, copyErr = nil, preserveErr + end CacheFs.prefix = savedPrefix if not copied then cleanup() diff --git a/src/mods/Loader.lua b/src/mods/Loader.lua index 8854b9bb..6875f38a 100644 --- a/src/mods/Loader.lua +++ b/src/mods/Loader.lua @@ -4,6 +4,7 @@ local SaveData = require("src.core.SaveData") local Data = require("src.core.Data") local GameVersion = require("src.core.GameVersion") local Version = require("src.core.Version") +local RequiredImports = require("src.mods.RequiredImports") local Assets = require("src.render.Assets") local ModUI = require("src.ui.ModUI") local DateTime = require("src.core.DateTime") @@ -565,7 +566,24 @@ function Loader:_validate() elseif manifest.assets_transforms and not self:_exists(mod.path .. "/" .. manifest.assets_transforms) then reason = "assets_transforms file missing: " .. manifest.assets_transforms - elseif manifest.game_version and not devEngine() then + end + if not reason and #(manifest.required_imports or {}) > 0 then + for _, import in ipairs(manifest.required_imports) do + local path = mod.path .. "/baseroms/" .. import.file + if not self:_exists(path) then + reason = "required import missing: " .. import.name + break + end + local data = self.fs.read and self.fs.read(path) + local valid, importErr = RequiredImports.validateStoredData(import, data) + if not valid then + reason = "required import invalid: " .. import.name + .. " (" .. tostring(importErr) .. ")" + break + end + end + end + if not reason and manifest.game_version and not devEngine() then local ok, err = Semver.satisfies(Version.engine, manifest.game_version) if not ok then reason = ("needs game version %s, engine is %s") diff --git a/src/mods/Manifest.lua b/src/mods/Manifest.lua index 95383cde..78bba2b9 100644 --- a/src/mods/Manifest.lua +++ b/src/mods/Manifest.lua @@ -132,13 +132,74 @@ local function mergeConflictLists(conflicts, incompatible) return out end +local scrubUtf8 -- shared by required-import labels and top-level strings + +-- User-supplied files a mod needs beside its own source. The launcher owns +-- the picker and the copy; the mod only reads the resulting +-- /baseroms/ through its existing scoped mod:read surface. MD5 is +-- deliberately the manifest vocabulary here: ROM preservation databases and +-- the mods consuming these files commonly identify dumps by MD5, and the +-- digest is an identity check rather than a security boundary. +local function parseImports(value, field, required) + field = field or "required_imports" + local out, ids, files = {}, {}, {} + for _, entry in ipairs(array(value)) do + assert(type(entry) == "table", field .. " entries must be objects") + local id = entry.id + assert(type(id) == "string" and id:match("^[%w_%-]+$"), + field .. " id must contain only letters, numbers, _ or -") + assert(not ids[id], "duplicate " .. field .. " id: " .. id) + ids[id] = true + + local file = entry.file + assert(type(file) == "string" and file ~= "", + field .. " file is required") + file = SafePath.require(file, field .. " file") + assert(not file:find("/", 1, true), + field .. " file must be a filename inside baseroms") + assert(not files[file], "duplicate " .. field .. " file: " .. file) + files[file] = true + + local hashes = entry.md5 + if type(hashes) == "string" then hashes = { hashes } end + assert(type(hashes) == "table" and #hashes > 0, + field .. " md5 must be a hash or non-empty array") + local accepted, seen = {}, {} + for _, digest in ipairs(hashes) do + assert(type(digest) == "string" and digest:match("^[%x]+$") + and #digest == 32, field .. " md5 values must be 32 hex characters") + digest = digest:lower() + if not seen[digest] then + seen[digest] = true + accepted[#accepted + 1] = digest + end + end + + local format = entry.format or "raw" + assert(format == "raw" or format == "n64", + field .. " format must be raw or n64") + local name = entry.name or id + assert(type(name) == "string" and name ~= "", + field .. " name must be a non-empty string") + out[#out + 1] = { + id = id, + name = scrubUtf8(name), + file = file, + md5 = accepted, + format = format, + required = required ~= false, + } + end + return out +end + -- Drop bytes that are not valid UTF-8 (malformed sequences, overlongs, -- surrogates, > U+10FFFF) and a leading BOM. LÖVE's text renderer raises -- "Invalid UTF-8" from love.graphics.print/printf, so any manifest string a -- panel may draw must be scrubbed here -- the one place every mod manifest -- passes through -- or a single mangled description crashes the whole MODS -- panel instead of misrendering one card. -local function scrubUtf8(s) +scrubUtf8 = function(s) if type(s) ~= "string" then return s end s = s:gsub("^\239\187\191", "") local out, i, n = {}, 1, #s @@ -290,6 +351,19 @@ function Manifest.validate(raw, path) local conflicts = mergeConflictLists(raw.conflicts, raw.incompatible) + local requiredImports = parseImports(raw.required_imports, + "required_imports", true) + local optionalImports = parseImports(raw.optional_imports, + "optional_imports", false) + local importIds, importFiles = {}, {} + for _, list in ipairs({ requiredImports, optionalImports }) do + for _, import in ipairs(list) do + assert(not importIds[import.id], "duplicate import id: " .. import.id) + assert(not importFiles[import.file], "duplicate import file: " .. import.file) + importIds[import.id], importFiles[import.file] = true, true + end + end + return { id = raw.id, name = raw.name, @@ -318,6 +392,8 @@ function Manifest.validate(raw, path) permissionSet = permissionSet, options_schema = optionalFile(raw.options_schema, "options_schema"), assets_transforms = optionalFile(raw.assets_transforms, "assets_transforms"), + required_imports = requiredImports, + optional_imports = optionalImports, -- an env var name, not a path, so it keeps the plain string check force_enable_env = optionalString(raw.force_enable_env, "force_enable_env"), path = path, diff --git a/src/mods/RequiredImports.lua b/src/mods/RequiredImports.lua new file mode 100644 index 00000000..3904da3f --- /dev/null +++ b/src/mods/RequiredImports.lua @@ -0,0 +1,266 @@ +-- Engine-owned import handling for files declared by a mod's required or +-- optional import arrays. Mods never receive host paths or broader +-- filesystem access: accepted bytes are copied into their own +-- mods//baseroms/ tree, where the existing mod:read sandbox can see them. + +local CacheFs = require("src.import.CacheFs") + +local RequiredImports = {} + +local function allSpecs(manifest) + local out = {} + for _, spec in ipairs((manifest and manifest.required_imports) or {}) do + out[#out + 1] = spec + end + for _, spec in ipairs((manifest and manifest.optional_imports) or {}) do + out[#out + 1] = spec + end + return out +end + +local function isRequired(spec) + return spec.required ~= false +end + +RequiredImports.specs = allSpecs + +local N64_MAGIC = { + ["\128\55\18\64"] = "z64", -- big endian / canonical + ["\55\128\64\18"] = "v64", -- byte-swapped + ["\64\18\55\128"] = "n64", -- little endian words +} + +local function n64KindAt(data, offset) + return N64_MAGIC[data:sub(offset, offset + 3)] +end + +-- Return canonical big-endian N64 bytes. A 512-byte copier header is +-- recognized only when valid N64 magic follows it, so arbitrary data is never +-- shortened just because its size happens to line up. +function RequiredImports.normalizeN64(data) + if type(data) ~= "string" then return nil, "selected file could not be read" end + local offset, kind = 1, n64KindAt(data, 1) + if not kind then + kind = n64KindAt(data, 513) + if kind then offset = 513 end + end + if not kind then return nil, "not a recognized Nintendo 64 ROM" end + data = data:sub(offset) + if kind == "z64" then return data end + + if kind == "v64" then + if #data % 2 ~= 0 then return nil, "byte-swapped N64 ROM has an odd size" end + return (data:gsub("(.)(.)", "%2%1")) + else + if #data % 4 ~= 0 then return nil, "little-endian N64 ROM size is not word aligned" end + return (data:gsub("(.)(.)(.)(.)", "%4%3%2%1")) + end +end + +function RequiredImports.normalize(spec, data) + if spec and spec.format == "n64" then + return RequiredImports.normalizeN64(data) + end + if type(data) ~= "string" then return nil, "selected file could not be read" end + return data +end + +local function hexDigest(data, hashFn) + if hashFn then return hashFn(data):lower() end + if not (love and love.data and love.data.hash and love.data.encode) then + return nil, "MD5 support is unavailable in this build" + end + local digest = love.data.hash("md5", data) + if type(digest) == "userdata" and digest.getString then + digest = digest:getString() + end + return love.data.encode("string", "hex", digest):lower() +end + +local function accepts(spec, digest) + for _, wanted in ipairs((spec and spec.md5) or {}) do + if wanted == digest then return true end + end + return false +end + +function RequiredImports.path(manifest, spec) + return manifest.path .. "/baseroms/" .. spec.file +end + +local function removedMarker(manifest, spec) + return manifest.path .. "/baseroms/." .. spec.id .. ".removed" +end + +-- Validate bytes against a declaration. The returned data is canonicalized +-- (notably for N64 byte order/header variants) and is what must be stored. +function RequiredImports.validateData(spec, data, hashFn) + local normalized, normalizeErr = RequiredImports.normalize(spec, data) + if not normalized then return nil, normalizeErr end + local digest, hashErr = hexDigest(normalized, hashFn) + if not digest then return nil, hashErr end + if not accepts(spec, digest) then + return nil, ("MD5 mismatch (got %s)"):format(digest) + end + return normalized, digest +end + +function RequiredImports.validateStoredData(spec, data, hashFn) + local normalized, detail = RequiredImports.validateData(spec, data, hashFn) + if not normalized then return nil, detail end + if normalized ~= data then + return nil, "stored N64 ROM is not canonical; choose the source file again" + end + return normalized, detail +end + +function RequiredImports.inspect(manifest, fs, hashFn) + fs = fs or (love and love.filesystem) + local rows, missing, missingOptional = {}, 0, 0 + for _, spec in ipairs(allSpecs(manifest)) do + local path = RequiredImports.path(manifest, spec) + local suppressed = fs and fs.getInfo + and fs.getInfo(removedMarker(manifest, spec), "file") ~= nil + local data = fs and fs.read and fs.read(path) or nil + local normalized, detail + if data then + normalized, detail = RequiredImports.validateStoredData(spec, data, hashFn) + end + local row = { id = spec.id, name = spec.name, file = spec.file, + format = spec.format, path = path, present = normalized ~= nil, + digest = normalized and detail or nil, + error = data and not normalized and detail or nil, + suppressed = suppressed, required = isRequired(spec), spec = spec } + if not row.present then + if row.required then missing = missing + 1 + else missingOptional = missingOptional + 1 end + end + rows[#rows + 1] = row + end + return rows, missing, missingOptional +end + +function RequiredImports.importData(manifest, importId, data, opts) + opts = opts or {} + local spec + for _, candidate in ipairs(allSpecs(manifest)) do + if candidate.id == importId then spec = candidate break end + end + if not spec then return nil, "unknown required import: " .. tostring(importId) end + local normalized, digest = RequiredImports.validateData(spec, data, opts.hash) + if not normalized then return nil, digest end + local savedPrefix = CacheFs.prefix + CacheFs.prefix = "" + local ok, err = CacheFs.write(RequiredImports.path(manifest, spec), normalized) + if ok then CacheFs.remove(removedMarker(manifest, spec)) end + CacheFs.prefix = savedPrefix + if not ok then return nil, "could not copy import: " .. tostring(err) end + return true, digest +end + +function RequiredImports.remove(manifest, importId) + for _, spec in ipairs(allSpecs(manifest)) do + if spec.id == importId then + local savedPrefix = CacheFs.prefix + CacheFs.prefix = "" + CacheFs.remove(RequiredImports.path(manifest, spec)) + local marked, markErr = CacheFs.write(removedMarker(manifest, spec), "removed\n") + CacheFs.prefix = savedPrefix + if not marked then return nil, "could not remember removal: " .. tostring(markErr) end + return true + end + end + return nil, "unknown required import: " .. tostring(importId) +end + +-- Fill missing imports from another installed mod when its accepted canonical +-- MD5 overlaps. The source remains inside the engine-owned mods tree, and a +-- fresh validation is performed before every copy. +function RequiredImports.reconcile(manifests, fs, hashFn) + fs = fs or (love and love.filesystem) + if not (fs and fs.read) then return {}, {} end + local available, state, declaredPaths = {}, {}, {} + for _, manifest in ipairs(manifests or {}) do + local rows, missing, missingOptional = {}, 0, 0 + for _, spec in ipairs(allSpecs(manifest)) do + local path = RequiredImports.path(manifest, spec) + declaredPaths[path] = true + local data = fs.read(path) + local suppressed = fs.getInfo + and fs.getInfo(removedMarker(manifest, spec), "file") ~= nil + local normalized, detail + if data then + normalized, detail = RequiredImports.validateStoredData(spec, data, hashFn) + if normalized then available[detail] = normalized end + end + local row = { id = spec.id, name = spec.name, file = spec.file, + format = spec.format, path = path, present = normalized ~= nil, + digest = normalized and detail or nil, + error = data and not normalized and detail or nil, + suppressed = suppressed, required = isRequired(spec), spec = spec } + if not row.present then + if row.required then missing = missing + 1 + else missingOptional = missingOptional + 1 end + end + rows[#rows + 1] = row + end + state[manifest.id] = { rows = rows, missing = missing, + missingOptional = missingOptional } + end + + + -- Compatibility with mods that already maintained their own baseroms + -- folder before this manifest field existed: index every other file in an + -- installed mod's folder by both raw and (when recognizable) canonical N64 + -- MD5. Nothing outside the engine-owned mods tree is searched. + if fs.getDirectoryItems and fs.getInfo then + for _, manifest in ipairs(manifests or {}) do + local dir = manifest.path .. "/baseroms" + if fs.getInfo(dir, "directory") then + for _, name in ipairs(fs.getDirectoryItems(dir) or {}) do + local path = dir .. "/" .. name + if name:sub(1, 1) ~= "." and not declaredPaths[path] + and fs.getInfo(path, "file") then + local data = fs.read(path) + if data then + local rawDigest = hexDigest(data, hashFn) + if rawDigest then available[rawDigest] = data end + local canonical = RequiredImports.normalizeN64(data) + if canonical then + local canonicalDigest = hexDigest(canonical, hashFn) + if canonicalDigest then available[canonicalDigest] = canonical end + end + end + end + end + end + end + end + + local copied = {} + for _, manifest in ipairs(manifests or {}) do + local entry = state[manifest.id] + for _, row in ipairs(entry.rows) do + if not row.present and not row.suppressed then + for _, digest in ipairs(row.spec.md5) do + local data = available[digest] + if data then + local ok = RequiredImports.importData(manifest, row.id, data, + { hash = hashFn }) + if ok then + copied[#copied + 1] = { mod = manifest.id, import = row.id, + digest = digest } + row.present, row.digest, row.error = true, digest, nil + if row.required then entry.missing = entry.missing - 1 + else entry.missingOptional = entry.missingOptional - 1 end + end + break + end + end + end + end + end + return copied, state +end + +return RequiredImports diff --git a/tests/launcher_mods_install_zip_test.lua b/tests/launcher_mods_install_zip_test.lua index e7b117f9..fc73a186 100644 --- a/tests/launcher_mods_install_zip_test.lua +++ b/tests/launcher_mods_install_zip_test.lua @@ -171,6 +171,16 @@ eq(staged, 0, "FileData path leaves no staged temp zip") check(files["mods/" .. MOD_ID .. "/manifest.json"] ~= nil, "install wrote manifest into mods/") +-- A third-party archive cannot bypass modkit's no-baseroms packaging gate. +resetFs() +ARCHIVE[MOD_ID .. "/baseroms/source.z64"] = "packaged rom" +files["imports/mods/packaged-rom.zip"] = "PK\3\4packaged-rom" +ok, err = LauncherMods.installZip("imports/mods/packaged-rom.zip") +check(not ok, "an archive containing baseroms is rejected") +check(tostring(err):find("must not include", 1, true), + "baseroms archive rejection explains the policy") +ARCHIVE[MOD_ID .. "/baseroms/source.z64"] = nil + -- Fallback: no newFileData → stage temp + path mount resetFs() vfs.newFileData = nil @@ -205,6 +215,23 @@ check(files["mods/WildsOfKanto-1.5.0/manifest.json"] == nil, check(files["mods/" .. MOD_ID .. "/manifest.json"] ~= nil, "replace still lands in mods/") +-- User-selected baseroms belong to the installation, not the downloaded mod +-- archive, and survive the same replacement path. +resetFs() +files["mods/OldFolder/manifest.json"] = + ('{"id":"%s","name":"Old Copy","version":"0.9.0","entry":"main.lua"}') + :format(MOD_ID) +files["mods/OldFolder/main.lua"] = "return function() end\n" +files["mods/OldFolder/baseroms/stadium2.z64"] = "user-owned-rom" +files["imports/mods/update-with-rom.zip"] = "PK\3\4update" +ok, err = LauncherMods.installZip("imports/mods/update-with-rom.zip", + { replace = true, expectId = MOD_ID }) +check(ok == true, "replace with a baserom succeeds (" .. tostring(err) .. ")") +eq(files["mods/" .. MOD_ID .. "/baseroms/stadium2.z64"], "user-owned-rom", + "replace preserves user-owned baseroms under the canonical mod folder") +check(files["mods/OldFolder/baseroms/stadium2.z64"] == nil, + "the shadow mod tree is still removed after preservation") + -- #834: a manifest-less mods/ tree (interrupted copy debris) must not -- block a plain re-import as "already installed" resetFs() diff --git a/tests/mod_required_imports_tests.lua b/tests/mod_required_imports_tests.lua new file mode 100644 index 00000000..2d068508 --- /dev/null +++ b/tests/mod_required_imports_tests.lua @@ -0,0 +1,193 @@ +package.path = "./?.lua;./?/init.lua;" .. package.path +love = love or require("tests.love_stub") + +local Manifest = require("src.mods.Manifest") +local RequiredImports = require("src.mods.RequiredImports") +local Loader = require("src.mods.Loader") +local S = require("tests.harness").suite("required mod imports") +local check, eq = S.check, S.eq + +local DIGEST = "0123456789abcdef0123456789abcdef" +local function fakeHash(data) + return data:sub(1, 4) == "\128\55\18\64" and DIGEST + or "ffffffffffffffffffffffffffffffff" +end + +local manifest = Manifest.validate({ + id = "stadium_fx", name = "Stadium FX", version = "1.0.0", entry = "main.lua", + required_imports = { + { id = "stadium2", name = "Stadium 2", file = "stadium2.z64", + format = "n64", md5 = { DIGEST, DIGEST:upper() } }, + }, +}, "mods/stadium_fx") + +eq(#manifest.required_imports, 1, "required import parses") +eq(#manifest.required_imports[1].md5, 1, "accepted MD5 values normalize and dedupe") +eq(manifest.required_imports[1].md5[1], DIGEST, "MD5 is lowercase") + +local optionalManifest = Manifest.validate({ + id = "optional_fx", name = "Optional FX", version = "1.0.0", entry = "main.lua", + optional_imports = { + { id = "bonus", name = "Bonus ROM", file = "bonus.z64", + format = "n64", md5 = DIGEST }, + }, +}, "mods/optional_fx") +eq(#optionalManifest.optional_imports, 1, "optional import parses") +eq(optionalManifest.optional_imports[1].required, false, + "optional import is marked non-blocking") +local optionalRows, optionalMissing, missingOptional = + RequiredImports.inspect(optionalManifest, love.filesystem, fakeHash) +eq(optionalMissing, 0, "missing optional import is not required") +eq(missingOptional, 1, "missing optional import is reported separately") +check(not optionalRows[1].required, "optional row is labeled optional") + +check(not pcall(Manifest.validate, { + id = "bad", name = "Bad", version = "1", entry = "main.lua", + required_imports = { { id = "rom", file = "../outside.z64", md5 = DIGEST } }, +}), "required import cannot escape baseroms") +check(not pcall(Manifest.validate, { + id = "bad", name = "Bad", version = "1", entry = "main.lua", + required_imports = { { id = "rom", file = "rom.z64", md5 = "short" } }, +}), "malformed MD5 is refused") + +local canonical = "\128\55\18\64ABCD" +local v64 = "\55\128\64\18BADC" +local n64 = "\64\18\55\128DCBA" +check(RequiredImports.importData(optionalManifest, "bonus", canonical, + { hash = fakeHash }), "optional import uses the normal validation path") +eq(RequiredImports.normalizeN64(canonical), canonical, "z64 stays canonical") +eq(RequiredImports.normalizeN64(v64), canonical, "v64 pair swap canonicalizes") +eq(RequiredImports.normalizeN64(n64), canonical, "n64 word swap canonicalizes") +eq(RequiredImports.normalizeN64(string.rep("H", 512) .. v64), canonical, + "recognized 512-byte copier header is stripped") +check(RequiredImports.normalizeN64(string.rep("H", 520)) == nil, + "an arbitrary 512-byte prefix is not treated as a copier header") + +local ok, digest = RequiredImports.importData(manifest, "stadium2", v64, + { hash = fakeHash }) +check(ok, "validated bytes import") +eq(digest, DIGEST, "import reports canonical digest") +eq(love.filesystem.read("mods/stadium_fx/baseroms/stadium2.z64"), canonical, + "import writes canonical bytes inside the mod") +local rows, missing = RequiredImports.inspect(manifest, love.filesystem, fakeHash) +eq(missing, 0, "written import satisfies its declaration") +check(rows[1].present, "inspection reports ready") + +local target = Manifest.validate({ + id = "other_fx", name = "Other FX", version = "1.0.0", entry = "main.lua", + required_imports = { + { id = "same_rom", file = "source.z64", format = "n64", md5 = DIGEST }, + }, +}, "mods/other_fx") +local copied = RequiredImports.reconcile({ manifest, target }, love.filesystem, fakeHash) +eq(#copied, 1, "matching installed import is reused") +eq(love.filesystem.read("mods/other_fx/baseroms/source.z64"), canonical, + "reuse creates a private per-mod copy") +check(RequiredImports.remove(target, "same_rom"), "a required import can be removed") +eq(love.filesystem.read("mods/other_fx/baseroms/source.z64"), nil, + "remove deletes this mod's private copy") +local copiedAfterRemove = RequiredImports.reconcile({ manifest, target }, + love.filesystem, fakeHash) +eq(#copiedAfterRemove, 0, + "an explicit removal is not immediately undone by automatic reuse") +check(RequiredImports.importData(target, "same_rom", canonical, { hash = fakeHash }), + "choosing the file again clears the removal decision") + +local legacy = Manifest.validate({ + id = "legacy", name = "Legacy", version = "1.0.0", entry = "main.lua", +}, "mods/legacy") +local legacyTarget = Manifest.validate({ + id = "legacy_user", name = "Legacy User", version = "1.0.0", entry = "main.lua", + required_imports = { + { id = "rom", file = "legacy-source.z64", format = "n64", md5 = DIGEST }, + }, +}, "mods/legacy_user") +love.filesystem.write("mods/legacy/baseroms/manually-imported.v64", v64) +local legacyCopied = RequiredImports.reconcile({ legacy, legacyTarget }, + love.filesystem, fakeHash) +eq(#legacyCopied, 1, "an undeclared legacy baserom can satisfy a new declaration") +eq(love.filesystem.read("mods/legacy_user/baseroms/legacy-source.z64"), canonical, + "legacy reuse still stores canonical bytes") + +local rejected, why = RequiredImports.importData(target, "same_rom", "wrong", + { hash = fakeHash }) +eq(rejected, nil, "mismatched selection is rejected") +check(tostring(why):find("Nintendo 64", 1, true) ~= nil, + "normalization failure explains the selected format") + +love.filesystem.write("mods/launcher_needs/manifest.json", ([[{ + "id":"launcher_needs","name":"Launcher Needs","version":"1.0.0", + "entry":"main.lua","required_imports":[{"id":"source","name":"Source ROM", + "file":"source.bin","md5":"%s"}] +}]]):format(DIGEST)) +love.filesystem.write("mods/launcher_needs/main.lua", "return function(mod) end") +local launcherRows = require("src.mods.LauncherMods").list() +eq(#launcherRows, 1, "launcher keeps a mod with a missing required import visible") +eq(launcherRows[1].missingRequiredImports, 1, + "launcher row carries the missing import count") +eq(launcherRows[1].status, "needs_import", + "missing import changes Ready to Import required") +check(launcherRows[1].statusDetail:find("Source ROM", 1, true) ~= nil, + "launcher warning names the missing file") + +local function memfs(files) + return { + read = function(path) return files[path] end, + getInfo = function(path) + if files[path] then return { type = "file" } end + local prefix = path .. "/" + for key in pairs(files) do + if key:sub(1, #prefix) == prefix then return { type = "directory" } end + end + end, + getDirectoryItems = function(path) + if path == "mods" then + local out, seen = {}, {} + for key in pairs(files) do + local id = key:match("^mods/([^/]+)/manifest%.json$") + if id and not seen[id] then seen[id] = true; out[#out + 1] = id end + end + table.sort(out) + return out + end + return {} + end, + load = function(path) + local source = files[path] + if not source then return nil, "missing" end + return load(source, path) + end, + } +end + +local manifestJson = ([[{ + "id":"needs_rom","name":"Needs ROM","version":"1.0.0","entry":"main.lua", + "required_imports":[{"id":"rom","name":"Source ROM","file":"source.bin", + "md5":"%s"}] +}]]):format(DIGEST) +local loader = Loader.new({ fs = memfs({ + ["mods/needs_rom/manifest.json"] = manifestJson, + ["mods/needs_rom/main.lua"] = "return function(mod) mod.exports.ran = true end", +}) }) +check(loader:load({}) == false, "missing required import blocks the enabled mod") +local status = loader:status().available[1] +eq(status.state, "invalid", "blocked mod reports invalid") +check(status.error:find("Source ROM", 1, true) ~= nil, + "loader failure names the required import") +check(not (loader.exports.needs_rom and loader.exports.needs_rom.ran), + "blocked mod entry never executes") + +local optionalJson = ([[{ + "id":"optional_rom","name":"Optional ROM","version":"1.0.0","entry":"main.lua", + "optional_imports":[{"id":"rom","name":"Bonus ROM","file":"bonus.bin", + "md5":"%s"}] +}]]):format(DIGEST) +local optionalLoader = Loader.new({ fs = memfs({ + ["mods/optional_rom/manifest.json"] = optionalJson, + ["mods/optional_rom/main.lua"] = "return function(mod) mod.exports.ran = true end", +}) }) +check(optionalLoader:load({}), "missing optional import does not block the mod") +check(optionalLoader.exports.optional_rom.ran, + "mod entry executes without its optional import") + +S.finish() diff --git a/tests/modkit_tests.lua b/tests/modkit_tests.lua index fc5758cc..ee53aef0 100644 --- a/tests/modkit_tests.lua +++ b/tests/modkit_tests.lua @@ -463,6 +463,8 @@ return function(mod) end ]]) write(bad .. "/hack.gb", "GBDATA") +os.execute((mkdir .. " %q"):format(bad .. "/baseroms")) +write(bad .. "/baseroms/stadium2.z64", "USER ROM") write(bad .. "/cachepath.lua", 'return { pic = "assets/generated/battle/front/mew.png" }') @@ -474,6 +476,8 @@ check(out:find("MK101", 1, true) ~= nil, "schema typo reported as MK101") check(out:find("base_stats", 1, true) ~= nil, "MK101 names the bad field") check(out:find("MK301", 1, true) ~= nil, "cache reference reported as MK301") check(out:find("MK303", 1, true) ~= nil, "ROM patch file reported as MK303") +check(out:find("MK307", 1, true) ~= nil, + "a user-supplied baseroms file is refused explicitly") out, code = run(("%s tools/modkit.py pack %q -o %q --base fixture") :format(python, bad, root .. "/bad.modpkg")) diff --git a/tests/rom_importer_android_mod_pick_test.lua b/tests/rom_importer_android_mod_pick_test.lua index 6e06fece..f863687e 100644 --- a/tests/rom_importer_android_mod_pick_test.lua +++ b/tests/rom_importer_android_mod_pick_test.lua @@ -14,6 +14,7 @@ love.system = love.system or {} local saved = { getOS = love.system.getOS, pickFile = love.system.pickFile, + pickFileKinds = love.system.pickFileKinds, } local pickCalls = {} @@ -22,6 +23,7 @@ love.system.pickFile = function(kind) pickCalls[#pickCalls + 1] = kind or "rom" return true end +love.system.pickFileKinds = function() return "rom,mod,sav,required_import" end local function freshImporter(ready) return setmetatable({ @@ -94,11 +96,65 @@ eq(ri._imported.source, "picked_save.sav", "focus reads the SAF save filename") check(love.filesystem.getInfo("picked_save.sav") == nil, "successful focus import removes picked_save.sav") +-- Required mod files use their own safe picker kind and pending filename. +pickCalls = {} +ri = freshImporter({ red = true, blue = true }) +ri.nativePicker = true +ri.mobileFileBridge = true +ri.mods = { { + id = "needs_source", + manifest = { id = "needs_source", name = "Needs Source", + required_imports = { { id = "source", name = "Source", file = "source.bin", + format = "raw", md5 = { "00000000000000000000000000000000" } } } }, +} } +ri:chooseRequiredImport("needs_source", "source") +eq(pickCalls[1], "required_import", + "required file asks for the dedicated picker kind") +eq(ri.pickerPendingModId, "needs_source", "pending mod is remembered") +eq(ri.pickerPendingImportId, "source", "pending import is remembered") + +-- A rejected selection stays on the imported-files page, where the player can +-- see it before choosing another file, instead of behind the modal. +ri.nativePicker = false +ri._importRequiredData = RomImporter._importRequiredData +local savedData = love.data +love.data = { + hash = function() return "not accepted" end, + encode = function() return "ffffffffffffffffffffffffffffffff" end, +} +ri:_importRequiredData("needs_source", "source", "wrong source bytes") +love.data = savedData +check(ri.requiredImportNotice ~= nil, + "required import rejection creates an in-modal notice") +eq(ri.requiredImportNotice.modId, "needs_source", + "required import notice identifies its mod") +eq(ri.requiredImportNotice.importId, "source", + "required import notice identifies its file") +check(ri.requiredImportNotice.text:find("MD5 mismatch", 1, true) ~= nil, + "required import notice includes the MD5 failure") +check(ri.modNotice == nil, + "required import rejection is not hidden in the general Mods notice") + +ri.nativePicker = true +ri._importRequiredSource = function(self, modId, importId, source) + self._requiredImported = { modId = modId, importId = importId, source = source } + return true +end +love.filesystem.write("picked_required_import.bin", "source bytes") +ri:focus(true) +check(ri._requiredImported ~= nil, "focus consumes a required-file SAF pick") +eq(ri._requiredImported.modId, "needs_source", "focus routes to the pending mod") +eq(ri._requiredImported.importId, "source", "focus routes to the pending declaration") +check(love.filesystem.getInfo("picked_required_import.bin") == nil, + "focus removes the staged required-file pick") + love.system.getOS = saved.getOS love.system.pickFile = saved.pickFile +love.system.pickFileKinds = saved.pickFileKinds -- leftover cleanup if a failed assertion left files behind love.filesystem.remove("usb_mod.zip") love.filesystem.remove("picked_mod.zip") love.filesystem.remove("picked_save.sav") +love.filesystem.remove("picked_required_import.bin") S.finish() diff --git a/tools/modkit.py b/tools/modkit.py index 42fc5acd..64b212d5 100644 --- a/tools/modkit.py +++ b/tools/modkit.py @@ -1045,6 +1045,14 @@ def lint_dir(repo, mod_dir, manifest): for rel in mod_files(mod_dir): path = os.path.join(mod_dir, rel) ext = os.path.splitext(rel)[1].lower() + # MK307: required_imports are user-owned installation state. Keeping + # baseroms in the walk without an explicit gate would let `pack` + # silently bundle exactly the ROM this feature exists not to ship. + if rel.startswith("baseroms/"): + findings.append(Finding( + "MK307", "error", + "user-supplied baseroms must not be distributed", rel)) + continue # MK301: nothing may live in (or point into) the generated trees if rel.startswith(("data/generated/", "assets/generated/")): findings.append(Finding( From dfc216f974fea35871e8417efa14a6d11fcf16c5 Mon Sep 17 00:00:00 2001 From: anxiousintrovert <82425472+anxiousintrovert@users.noreply.github.com> Date: Fri, 14 Aug 2026 14:30:45 -0500 Subject: [PATCH 2/3] Remove required imports RFC --- docs/rfcs/0010-required-mod-imports.md | 105 ------------------------- 1 file changed, 105 deletions(-) delete mode 100644 docs/rfcs/0010-required-mod-imports.md diff --git a/docs/rfcs/0010-required-mod-imports.md b/docs/rfcs/0010-required-mod-imports.md deleted file mode 100644 index 3196bee8..00000000 --- a/docs/rfcs/0010-required-mod-imports.md +++ /dev/null @@ -1,105 +0,0 @@ -# RFC 0010 — Manifest-declared user imports for mods - -## Status - -Proposed. Engine: `Manifest.lua`, `RequiredImports.lua`, `Loader.lua`, -`LauncherMods.lua`. Launcher: `RomImporter.lua`, `LauncherView.lua`. Native -picker bridges: Android and iOS. - -## Motivation - -Some mods derive presentation data from another cartridge the player owns. -Shipping those bytes in a mod is not acceptable, while asking each mod to -escape the sandbox, implement native pickers, and maintain platform-specific -paths defeats the sandbox's purpose. Pokemon Stadium and Pokemon Stadium 2 are -the first concrete consumers, but the ownership/import problem is generic. - -## The decision it extends - -This extends the manifest as the engine-owned declaration of mod dependencies -and preserves the sandbox rule in `src/mods/Sandbox.lua`: mods do not receive -raw host filesystem access. It also extends the launcher's established ROM, -save, and mod archive picker/inbox flows instead of introducing a second host -integration. - -## Exact manifest delta - -Additive `required_imports` and `optional_imports` arrays are accepted: - -```json -{ - "required_imports": [{ - "id": "stadium2", - "name": "Pokemon Stadium 2 ROM", - "file": "stadium2.z64", - "format": "n64", - "md5": ["00000000000000000000000000000000"] - }] -} -``` - -Both arrays use the same object schema. A missing `required_imports` entry -blocks the mod; a missing `optional_imports` entry only leaves that bonus -functionality unavailable. - -- `id` is unique within the manifest and uses the mod-id vocabulary. -- `name` is launcher-facing text. -- `file` is one safe filename below the mod's `baseroms/` directory. -- `md5` is one 32-digit hexadecimal digest or a non-empty array of accepted - digests. MD5 is a known-dump identity convention, not a trust primitive. -- `format` is `raw` by default. `n64` recognizes a canonical big-endian dump, - pair-byte-swapped and little-endian-word dumps, with or without a recognized - 512-byte copier header. Validation and stored output use canonical big-endian - bytes. - -The launcher copies a validated file to -`mods//baseroms/`. Missing required imports make the launcher -row need attention and make the loader refuse that enabled mod before its entry -chunk runs. Missing optional imports remain selectable without changing load -status. A matching validated import owned by another installed mod is -copied automatically. Replace and remove remain explicit per-mod actions. - -Desktop and UWP use their existing native/host picker routes. Android and iOS -add a `required_import` picker kind which stages -`picked_required_import.bin`. NX scans the engine-owned -`imports/baseroms/` MTP inbox. All writes continue through `CacheFs`, preserving -portable-mode placement. - -`modkit validate` and `modkit pack` report MK307 for every file beneath a -source mod's `baseroms/` directory, so the packaging path cannot accidentally -distribute a file the launcher placed there. The installer also rejects a mod -archive containing `baseroms/` files, covering packages built without modkit. - -## Mod-facing API and sandbox statement - -There is no new runtime API and no new permission. A mod reads its own copied -file through the existing `mod:read("baseroms/")` capability. It never -learns the selected host path, cannot browse another mod's tree, and receives -no raw `io`, `love.filesystem`, or platform-picker access. - -## Migration and compatibility - -Existing manifests omit both import arrays and behave exactly as before. The -fields are additive for both manifest API levels. Mods currently maintaining -their own cross-platform picker can declare the source file and remove that -host integration; their processing code changes only to read the declared -`baseroms/` path. - -## Parity tests - -- Empty/absent `required_imports` leaves existing manifests and loader behavior - unchanged. -- Manifest validation refuses path traversal, duplicate ids/files, malformed - MD5 values, and unknown normalization formats. -- N64 byte orders and the recognized copier-header form produce identical - canonical bytes before MD5 validation. -- A mismatched selection is never written. -- An enabled mod with a missing declared file never executes its entry chunk. -- A matching import in another installed mod is copied into the requesting - mod's own `baseroms/` directory. -- The packaging gate refuses every `baseroms/` file. -- The launcher refuses an archive that contains `baseroms/` files. - -## Deprecation etiquette - -Nothing deprecated or removed. From c48fc578ca9dae596c458599c725e72872c2cc27 Mon Sep 17 00:00:00 2001 From: anxiousintrovert <82425472+anxiousintrovert@users.noreply.github.com> Date: Fri, 14 Aug 2026 17:08:23 -0500 Subject: [PATCH 3/3] Address required import review feedback --- .gitignore | 1 + docs/modding.md | 28 ++- .../jni/love/src/modules/system/System.cpp | 2 + src/import/LauncherView.lua | 12 +- src/import/RomImporter.lua | 63 ++++- src/mods/LauncherMods.lua | 24 +- src/mods/Loader.lua | 4 +- src/mods/Manifest.lua | 23 ++ src/mods/RequiredImports.lua | 229 ++++++++++-------- tests/launcher_mods_install_zip_test.lua | 33 +++ tests/mod_required_imports_tests.lua | 91 +++++-- tests/rom_importer_android_mod_pick_test.lua | 18 ++ 12 files changed, 384 insertions(+), 144 deletions(-) diff --git a/.gitignore b/.gitignore index 0f509ed2..16d74675 100644 --- a/.gitignore +++ b/.gitignore @@ -84,3 +84,4 @@ mobile/ios/bundle_id.local # destinations, but source checkouts and packaged mods never ship the files. /mods/*/baseroms/ /imports/baseroms/ +/imports/baseroms-recovery/ diff --git a/docs/modding.md b/docs/modding.md index 621430dc..19c88f0b 100644 --- a/docs/modding.md +++ b/docs/modding.md @@ -48,8 +48,10 @@ Every mod contains a root `manifest.json` defining its metadata, supported games { "id": "stadium2", "name": "Pokemon Stadium 2 ROM", + "description": "Pokemon Stadium 2 (USA), any supported N64 byte order", "file": "stadium2.z64", "format": "n64", + "size": 67108864, "md5": ["00000000000000000000000000000000"] } ], @@ -116,21 +118,35 @@ When a mod supports multiple games (`"games": ["gen1", "gen2"]`), a dependency c out of mod archives while giving every platform the same installation flow. Each object requires a stable `id`, a display `name`, a destination `file` (a filename, never a path), and one MD5 digest or an array of accepted MD5 -digests. `format` is either `"raw"` (the default) or `"n64"`. +digests. `format` is either `"raw"` (the default) or `"n64"`. An optional +`description` gives players dump or region guidance in the import panel. +`size` declares the exact canonical byte length; `max_size` declares a smaller +per-import ceiling when an exact size is not appropriate. Every import also +has an engine-enforced 128 MiB ceiling and is rejected before hashing when its +filesystem reports an invalid size. For `"n64"`, the launcher recognizes `.z64`, `.v64`, and `.n64` byte orders, strips a recognized 512-byte copier header, converts the bytes to canonical big-endian `.z64` order, and then checks MD5. The canonical bytes are written -to `mods//baseroms/`. Another installed mod with an overlapping -accepted MD5 automatically supplies a copy, so the player only selects a ROM -once. Mods read the result with their existing scoped `mod:read` API, for +to `mods//baseroms/`. Each selection is a private grant to that +mod: the launcher never scans or copies another mod's imported files merely +because its manifest names the same digest. Mods read the result with their existing scoped `mod:read` API, for example `mod:read("baseroms/stadium2.z64")`; no host path or new filesystem permission is exposed. Missing `required_imports` block the mod before its entry chunk runs; missing `optional_imports` remain visible in the same launcher panel but do not block loading. -MD5 here identifies a known dump; it is not used as a security or authenticity -guarantee. Mod archives must not include anything beneath `baseroms/`. +MD5 here identifies a known dump because ROM databases commonly publish it; +it is not a security or authenticity guarantee. Do not paste the SHA-1 used by +Gen1Recomp's own game-ROM importer into an import's `md5` field. Mod archives +must not include anything beneath `baseroms/`. The engine records a validation +receipt keyed by file size and modification time so launcher refreshes and +later boots do not repeatedly hash an unchanged imported ROM. + +New mobile code should call `love.system.pickFile("required_import")`. The +older iOS-only `"stadium"` picker kind remains temporarily for compatibility. +Android now returns `false` for unknown picker kinds instead of treating them +as game-ROM picks. ## Mods and Gold (Gen 2) diff --git a/mobile/android/love/src/jni/love/src/modules/system/System.cpp b/mobile/android/love/src/jni/love/src/modules/system/System.cpp index e5f84b24..fce90d63 100644 --- a/mobile/android/love/src/jni/love/src/modules/system/System.cpp +++ b/mobile/android/love/src/jni/love/src/modules/system/System.cpp @@ -196,6 +196,8 @@ bool System::pickFile(const char *kind) const dest = "picked_required_import.bin"; else if (strcmp(kind, "rom") == 0) dest = "picked_rom.gb"; + // Unknown kinds used to fall through to the ROM destination. Refuse them + // so a newer Lua caller cannot silently route an unrelated file as a ROM. else return false; } diff --git a/src/import/LauncherView.lua b/src/import/LauncherView.lua index eb4a49d3..a8477d7e 100644 --- a/src/import/LauncherView.lua +++ b/src/import/LauncherView.lua @@ -2877,7 +2877,7 @@ local function buildRequiredImportsModal(imp, m) end local noticeW = w - 2 * pad local noticeH = noticeText and Kit.wrapHeight("small", noticeText, noticeW, 2) or 0 - local rowH = math.max(math.floor(56 * m.s), m.btnH) + local rowH = math.max(math.floor(70 * m.s), m.btnH) local perPage = math.min(4, math.max(1, #imports)) local pagerH = #imports > perPage and math.max(Kit.tapMin(), math.floor(30 * m.s)) or 0 local h = pad + Kit.textHeight("button") + math.floor(4 * m.s) @@ -2914,12 +2914,18 @@ local function buildRequiredImportsModal(imp, m) local textW = actionX - innerX - math.floor(8 * m.s) Kit.text("small", Kit.ellipsize("small", row.name, textW), innerX, cy + math.floor(8 * m.s), PAL.heading) + local stateY = cy + math.floor(8 * m.s) + Kit.textHeight("small") + + math.floor(3 * m.s) + if row.description and row.description ~= "" then + Kit.text("micro", Kit.ellipsize("micro", row.description, textW), + innerX, stateY, PAL.muted) + stateY = stateY + Kit.textHeight("micro") + math.floor(2 * m.s) + end local state = row.present and Strings("Ready - %s", row.file) or (row.error and Strings("Invalid file - choose again") or (row.required and Strings("Required - %s", row.file) or Strings("Optional - %s", row.file))) - Kit.text("micro", Kit.ellipsize("micro", state, textW), innerX, - cy + math.floor(8 * m.s) + Kit.textHeight("small") + math.floor(3 * m.s), + Kit.text("micro", Kit.ellipsize("micro", state, textW), innerX, stateY, row.present and PAL.green or (row.required and PAL.yellow or PAL.muted)) btn(imp, actionX, cy + (rowH - m.btnH) / 2, actionW, m.btnH, "req-pick-" .. mod.id .. "-" .. importId, diff --git a/src/import/RomImporter.lua b/src/import/RomImporter.lua index 3336ff5d..1b178af4 100644 --- a/src/import/RomImporter.lua +++ b/src/import/RomImporter.lua @@ -344,6 +344,14 @@ local function readExternalPath(path) return data end +local function externalFileSize(path) + local file = io.open(path, "rb") + if not file then return nil end + local size = file:seek("end") + file:close() + return size +end + local function readDroppedFile(file) local ok, openError = file:open("r") if not ok then return nil, openError end @@ -1131,11 +1139,11 @@ local function chooseSav() return nil end --- Generic user-supplied dependency picker. Validation is manifest-driven, --- so the dialog intentionally permits every file extension; a wrong choice --- cannot reach the mod because its canonical MD5 must match first. -local function chooseRequiredFile(label) - local prompt = shellSafe("Choose " .. tostring(label or "required file")) +-- Generic user-supplied dependency picker. Keep the native prompt entirely +-- engine-owned: manifest labels are untrusted and must never enter shell +-- command templates. The LÖVE modal already shows the specific import name. +local function chooseRequiredFile() + local prompt = shellSafe(Strings("Choose required mod file")) local platform = love.system.getOS() if platform == "OS X" then return commandOutput( @@ -1819,6 +1827,13 @@ local function requiredManifest(self, modId) return nil end +local function requiredSpec(manifest, importId) + for _, candidate in ipairs(require("src.mods.RequiredImports").specs(manifest)) do + if candidate.id == importId then return candidate end + end + return nil +end + local function requiredImportNotice(self, modId, importId, text) self.requiredImportNotice = { modId = modId, @@ -1850,6 +1865,21 @@ function RomImporter:_importRequiredData(modId, importId, data) end function RomImporter:_importRequiredSource(modId, importId, source) + local manifest = requiredManifest(self, modId) + local spec = manifest and requiredSpec(manifest, importId) + if not spec then + requiredImportNotice(self, modId, importId, "Import declaration was not found.") + self.modNotice = nil + return nil + end + local info = love.filesystem.getInfo(source, "file") + local size = info and info.size or externalFileSize(source) + local sizeErr = require("src.mods.RequiredImports").sizeError(spec, size, false) + if sizeErr then + requiredImportNotice(self, modId, importId, sizeErr) + self.modNotice = nil + return nil + end local data = love.filesystem.read(source) if not data then data = readExternalPath(source) end if not data then @@ -1881,24 +1911,31 @@ function RomImporter:chooseRequiredImport(modId, importId) if self.workState == "working" then return end local manifest = requiredManifest(self, modId) if not manifest then return end - local spec - for _, candidate in ipairs(require("src.mods.RequiredImports").specs(manifest)) do - if candidate.id == importId then spec = candidate break end - end + local spec = requiredSpec(manifest, importId) if not spec then return end if self.isNX then local inbox = "imports/baseroms" love.filesystem.createDirectory(inbox) + local lastError for _, name in ipairs(love.filesystem.getDirectoryItems(inbox) or {}) do if name:sub(1, 1) ~= "." then local path = inbox .. "/" .. name - local data = love.filesystem.read(path) + local info = love.filesystem.getInfo(path, "file") + local sizeErr = info and require("src.mods.RequiredImports") + .sizeError(spec, info.size, false) + local data = not sizeErr and love.filesystem.read(path) or nil if data and self:_importRequiredData(modId, importId, data) then return end + if sizeErr then lastError = sizeErr + elseif self.requiredImportNotice + and self.requiredImportNotice.modId == modId + and self.requiredImportNotice.importId == importId then + lastError = self.requiredImportNotice.text + end end end - requiredImportNotice(self, modId, importId, - "No matching file in imports/baseroms/. Copy it there over MTP, then try again.") + requiredImportNotice(self, modId, importId, lastError + or "No matching file in imports/baseroms/. Copy it there over MTP, then try again.") self.modNotice = nil return end @@ -1925,7 +1962,7 @@ function RomImporter:chooseRequiredImport(modId, importId) return end - local path = chooseRequiredFile(spec.name) + local path = chooseRequiredFile() if path then self:_importRequiredSource(modId, importId, path) end end diff --git a/src/mods/LauncherMods.lua b/src/mods/LauncherMods.lua index 6679d977..87e51acb 100644 --- a/src/mods/LauncherMods.lua +++ b/src/mods/LauncherMods.lua @@ -460,9 +460,14 @@ function LauncherMods.list(version) local ok, result = pcall(function() local options = SaveData.loadOptions() local manifests = discover() - -- A validated copy owned by another installed mod can satisfy the same - -- declared MD5 without asking the player to select the ROM twice. - local _, importState = RequiredImports.reconcile(manifests) + -- Imports are private player grants. Never scan or copy another mod's + -- baseroms: a matching public digest is not permission to share the file. + local importState = {} + for _, manifest in ipairs(manifests) do + local rows, missing, missingOptional = RequiredImports.inspect(manifest) + importState[manifest.id] = { rows = rows, missing = missing, + missingOptional = missingOptional } + end -- The first build containing game-specific switches turns the old shared -- state into one explicit answer per installed mod and game. Saving here -- means users who only visit the launcher still receive the migration. @@ -968,12 +973,16 @@ function LauncherMods._installZipInner(source, opts) end local dest = "mods/" .. manifest.id + local baseromRecovery = "imports/baseroms-recovery/" .. manifest.id local existing, installedSomewhere = sameIdTrees(fs, manifest.id) if installedSomewhere and not opts.replace then cleanup() return nil, "a mod named '" .. manifest.id .. "' is already installed" end local preservedBaseroms = {} + -- A previous failed update may have staged the user's files outside mods/ so + -- discovery cannot mistake recovery debris for an installed mod. + snapshotTree(baseromRecovery, preservedBaseroms) if #existing > 0 then for _, path in ipairs(existing) do snapshotTree(path .. "/baseroms", preservedBaseroms) @@ -1012,13 +1021,16 @@ function LauncherMods._installZipInner(source, opts) end end if preserveErr then - -- Do not report a successful update that discarded user-owned input. Keep - -- a best-effort baseroms-only tree for the next retry instead. + -- Do not report a successful update that discarded user-owned input, and + -- do not leave a manifest-less baseroms tree that resembles an install. removeTree(dest) + removeTree(baseromRecovery) for rel, bytes in pairs(preservedBaseroms) do - if bytes ~= nil then CacheFs.write(dest .. "/baseroms/" .. rel, bytes) end + if bytes ~= nil then CacheFs.write(baseromRecovery .. "/" .. rel, bytes) end end copied, copyErr = nil, preserveErr + elseif copied then + removeTree(baseromRecovery) end CacheFs.prefix = savedPrefix if not copied then diff --git a/src/mods/Loader.lua b/src/mods/Loader.lua index 6875f38a..1a46dbf9 100644 --- a/src/mods/Loader.lua +++ b/src/mods/Loader.lua @@ -574,8 +574,8 @@ function Loader:_validate() reason = "required import missing: " .. import.name break end - local data = self.fs.read and self.fs.read(path) - local valid, importErr = RequiredImports.validateStoredData(import, data) + local valid, importErr = RequiredImports.validateStored( + manifest, import, self.fs) if not valid then reason = "required import invalid: " .. import.name .. " (" .. tostring(importErr) .. ")" diff --git a/src/mods/Manifest.lua b/src/mods/Manifest.lua index 78bba2b9..1fef0295 100644 --- a/src/mods/Manifest.lua +++ b/src/mods/Manifest.lua @@ -157,6 +157,8 @@ local function parseImports(value, field, required) file = SafePath.require(file, field .. " file") assert(not file:find("/", 1, true), field .. " file must be a filename inside baseroms") + assert(file:sub(1, 1) ~= ".", + field .. " file must not use a hidden metadata filename") assert(not files[file], "duplicate " .. field .. " file: " .. file) files[file] = true @@ -181,12 +183,33 @@ local function parseImports(value, field, required) local name = entry.name or id assert(type(name) == "string" and name ~= "", field .. " name must be a non-empty string") + local description = entry.description or entry.hint + if description ~= nil then + assert(type(description) == "string" and description ~= "", + field .. " description must be a non-empty string") + end + local size = entry.size + local maxSize = entry.max_size + local function validateSize(value, label) + if value == nil then return end + assert(type(value) == "number" and value > 0 and value % 1 == 0, + field .. " " .. label .. " must be a positive integer") + assert(value <= 128 * 1024 * 1024, + field .. " " .. label .. " exceeds the 128 MiB hard limit") + end + validateSize(size, "size") + validateSize(maxSize, "max_size") + assert(not (size and maxSize) or size <= maxSize, + field .. " size must not exceed max_size") out[#out + 1] = { id = id, name = scrubUtf8(name), + description = scrubUtf8(description), file = file, md5 = accepted, format = format, + size = size, + max_size = maxSize, required = required ~= false, } end diff --git a/src/mods/RequiredImports.lua b/src/mods/RequiredImports.lua index 3904da3f..167c263f 100644 --- a/src/mods/RequiredImports.lua +++ b/src/mods/RequiredImports.lua @@ -23,6 +23,34 @@ local function isRequired(spec) end RequiredImports.specs = allSpecs +RequiredImports.MAX_BYTES = 128 * 1024 * 1024 + +local function sizeLabel(bytes) + return ("%.1f MiB"):format(bytes / (1024 * 1024)) +end + +-- Check size before a caller reads an external or stored file into one large +-- Lua string. N64 sources may carry a 512-byte copier header, while stored +-- files are always canonical and therefore must match the declared size. +function RequiredImports.sizeError(spec, size, stored) + if type(size) ~= "number" then return nil end + if size > RequiredImports.MAX_BYTES then + return ("file is too large (%s; hard limit is %s)") + :format(sizeLabel(size), sizeLabel(RequiredImports.MAX_BYTES)) + end + local headerAllowance = not stored and spec and spec.format == "n64" and 512 or 0 + if spec and spec.size then + if size ~= spec.size and size ~= spec.size + headerAllowance then + return ("wrong file size (expected %d bytes%s, got %d)") + :format(spec.size, headerAllowance > 0 and " or a 512-byte header" or "", size) + end + end + if spec and spec.max_size and size > spec.max_size + headerAllowance then + return ("file is too large for this import (maximum %d bytes, got %d)") + :format(spec.max_size, size) + end + return nil +end local N64_MAGIC = { ["\128\55\18\64"] = "z64", -- big endian / canonical @@ -44,7 +72,9 @@ function RequiredImports.normalizeN64(data) kind = n64KindAt(data, 513) if kind then offset = 513 end end - if not kind then return nil, "not a recognized Nintendo 64 ROM" end + if not kind then + return nil, "expected an N64 ROM (.z64/.v64/.n64); file signature was not recognized" + end data = data:sub(offset) if kind == "z64" then return data end @@ -89,14 +119,71 @@ function RequiredImports.path(manifest, spec) end local function removedMarker(manifest, spec) - return manifest.path .. "/baseroms/." .. spec.id .. ".removed" + return manifest.path .. "/baseroms/.required-import-" .. spec.id .. ".removed" +end + +local function receiptPath(manifest, spec) + return manifest.path .. "/baseroms/.required-import-" .. spec.id .. ".validated" +end + +RequiredImports.receiptPath = receiptPath + +local function parseReceipt(raw) + if type(raw) ~= "string" then return nil end + local digest, size, modtime = raw:match("^v1\n([%x]+)\n(%d+)\n([^\n]+)\n?$") + if not digest then return nil end + return digest:lower(), tonumber(size), tonumber(modtime) +end + +local function cachedDigest(manifest, spec, fs, info) + -- A size alone cannot detect a same-length replacement. Require modtime as + -- well; filesystems that do not expose it simply take the safe hash path. + if not (fs and fs.read and info and info.size and info.modtime) then return nil end + local digest, size, modtime = parseReceipt(fs.read(receiptPath(manifest, spec))) + if digest and size == info.size and modtime == info.modtime + and accepts(spec, digest) then + return digest + end + return nil +end + +local function writeReceipt(manifest, spec, digest, info, fs) + if not (digest and info and info.size and info.modtime) then return end + local path = receiptPath(manifest, spec) + local body = ("v1\n%s\n%d\n%s\n") + :format(digest, info.size, tostring(info.modtime)) + if love and fs == love.filesystem then + local savedPrefix = CacheFs.prefix + CacheFs.prefix = "" + CacheFs.write(path, body) + CacheFs.prefix = savedPrefix + elseif fs and fs.write then + fs.write(path, body) + end +end + +local function removeReceipt(manifest, spec, fs) + local path = receiptPath(manifest, spec) + if love and fs == love.filesystem then + local savedPrefix = CacheFs.prefix + CacheFs.prefix = "" + CacheFs.remove(path) + CacheFs.prefix = savedPrefix + elseif fs and fs.remove then + fs.remove(path) + end end -- Validate bytes against a declaration. The returned data is canonicalized -- (notably for N64 byte order/header variants) and is what must be stored. function RequiredImports.validateData(spec, data, hashFn) + local sourceSizeErr = type(data) == "string" + and RequiredImports.sizeError(spec, #data, false) + if sourceSizeErr then return nil, sourceSizeErr end local normalized, normalizeErr = RequiredImports.normalize(spec, data) if not normalized then return nil, normalizeErr end + local storedSizeErr = RequiredImports.sizeError(spec, #normalized, true) + if storedSizeErr then return nil, storedSizeErr end local digest, hashErr = hexDigest(normalized, hashFn) if not digest then return nil, hashErr end if not accepts(spec, digest) then @@ -105,6 +192,35 @@ function RequiredImports.validateData(spec, data, hashFn) return normalized, digest end +-- Validate one installed import without reading it when the engine-authored +-- receipt still matches the file's size and modification time. +function RequiredImports.validateStored(manifest, spec, fs, hashFn) + fs = fs or (love and love.filesystem) + if not (fs and fs.getInfo) then return nil, "filesystem is unavailable" end + local path = RequiredImports.path(manifest, spec) + local info = fs.getInfo(path, "file") + if not info then + removeReceipt(manifest, spec, fs) + return nil, "file is missing" + end + local sizeErr = RequiredImports.sizeError(spec, info.size, true) + if sizeErr then + removeReceipt(manifest, spec, fs) + return nil, sizeErr + end + local cached = cachedDigest(manifest, spec, fs, info) + if cached then return true, cached, true end + removeReceipt(manifest, spec, fs) + if not fs.read then return nil, "file could not be read" end + local data = fs.read(path) + local normalized, detail = RequiredImports.validateStoredData(spec, data, hashFn) + if not normalized then return nil, detail end + info = fs.getInfo(path, "file") or info + info.size = info.size or #data + writeReceipt(manifest, spec, detail, info, fs) + return true, detail, false +end + function RequiredImports.validateStoredData(spec, data, hashFn) local normalized, detail = RequiredImports.validateData(spec, data, hashFn) if not normalized then return nil, detail end @@ -121,15 +237,12 @@ function RequiredImports.inspect(manifest, fs, hashFn) local path = RequiredImports.path(manifest, spec) local suppressed = fs and fs.getInfo and fs.getInfo(removedMarker(manifest, spec), "file") ~= nil - local data = fs and fs.read and fs.read(path) or nil - local normalized, detail - if data then - normalized, detail = RequiredImports.validateStoredData(spec, data, hashFn) - end + local exists = fs and fs.getInfo and fs.getInfo(path, "file") ~= nil + local valid, detail = RequiredImports.validateStored(manifest, spec, fs, hashFn) local row = { id = spec.id, name = spec.name, file = spec.file, - format = spec.format, path = path, present = normalized ~= nil, - digest = normalized and detail or nil, - error = data and not normalized and detail or nil, + description = spec.description, format = spec.format, path = path, + present = valid == true, digest = valid and detail or nil, + error = exists and not valid and detail or nil, suppressed = suppressed, required = isRequired(spec), spec = spec } if not row.present then if row.required then missing = missing + 1 @@ -151,8 +264,13 @@ function RequiredImports.importData(manifest, importId, data, opts) if not normalized then return nil, digest end local savedPrefix = CacheFs.prefix CacheFs.prefix = "" + CacheFs.remove(receiptPath(manifest, spec)) local ok, err = CacheFs.write(RequiredImports.path(manifest, spec), normalized) if ok then CacheFs.remove(removedMarker(manifest, spec)) end + if ok and love and love.filesystem and love.filesystem.getInfo then + local info = love.filesystem.getInfo(RequiredImports.path(manifest, spec), "file") + writeReceipt(manifest, spec, digest, info, love.filesystem) + end CacheFs.prefix = savedPrefix if not ok then return nil, "could not copy import: " .. tostring(err) end return true, digest @@ -164,6 +282,7 @@ function RequiredImports.remove(manifest, importId) local savedPrefix = CacheFs.prefix CacheFs.prefix = "" CacheFs.remove(RequiredImports.path(manifest, spec)) + CacheFs.remove(receiptPath(manifest, spec)) local marked, markErr = CacheFs.write(removedMarker(manifest, spec), "removed\n") CacheFs.prefix = savedPrefix if not marked then return nil, "could not remember removal: " .. tostring(markErr) end @@ -173,94 +292,4 @@ function RequiredImports.remove(manifest, importId) return nil, "unknown required import: " .. tostring(importId) end --- Fill missing imports from another installed mod when its accepted canonical --- MD5 overlaps. The source remains inside the engine-owned mods tree, and a --- fresh validation is performed before every copy. -function RequiredImports.reconcile(manifests, fs, hashFn) - fs = fs or (love and love.filesystem) - if not (fs and fs.read) then return {}, {} end - local available, state, declaredPaths = {}, {}, {} - for _, manifest in ipairs(manifests or {}) do - local rows, missing, missingOptional = {}, 0, 0 - for _, spec in ipairs(allSpecs(manifest)) do - local path = RequiredImports.path(manifest, spec) - declaredPaths[path] = true - local data = fs.read(path) - local suppressed = fs.getInfo - and fs.getInfo(removedMarker(manifest, spec), "file") ~= nil - local normalized, detail - if data then - normalized, detail = RequiredImports.validateStoredData(spec, data, hashFn) - if normalized then available[detail] = normalized end - end - local row = { id = spec.id, name = spec.name, file = spec.file, - format = spec.format, path = path, present = normalized ~= nil, - digest = normalized and detail or nil, - error = data and not normalized and detail or nil, - suppressed = suppressed, required = isRequired(spec), spec = spec } - if not row.present then - if row.required then missing = missing + 1 - else missingOptional = missingOptional + 1 end - end - rows[#rows + 1] = row - end - state[manifest.id] = { rows = rows, missing = missing, - missingOptional = missingOptional } - end - - - -- Compatibility with mods that already maintained their own baseroms - -- folder before this manifest field existed: index every other file in an - -- installed mod's folder by both raw and (when recognizable) canonical N64 - -- MD5. Nothing outside the engine-owned mods tree is searched. - if fs.getDirectoryItems and fs.getInfo then - for _, manifest in ipairs(manifests or {}) do - local dir = manifest.path .. "/baseroms" - if fs.getInfo(dir, "directory") then - for _, name in ipairs(fs.getDirectoryItems(dir) or {}) do - local path = dir .. "/" .. name - if name:sub(1, 1) ~= "." and not declaredPaths[path] - and fs.getInfo(path, "file") then - local data = fs.read(path) - if data then - local rawDigest = hexDigest(data, hashFn) - if rawDigest then available[rawDigest] = data end - local canonical = RequiredImports.normalizeN64(data) - if canonical then - local canonicalDigest = hexDigest(canonical, hashFn) - if canonicalDigest then available[canonicalDigest] = canonical end - end - end - end - end - end - end - end - - local copied = {} - for _, manifest in ipairs(manifests or {}) do - local entry = state[manifest.id] - for _, row in ipairs(entry.rows) do - if not row.present and not row.suppressed then - for _, digest in ipairs(row.spec.md5) do - local data = available[digest] - if data then - local ok = RequiredImports.importData(manifest, row.id, data, - { hash = hashFn }) - if ok then - copied[#copied + 1] = { mod = manifest.id, import = row.id, - digest = digest } - row.present, row.digest, row.error = true, digest, nil - if row.required then entry.missing = entry.missing - 1 - else entry.missingOptional = entry.missingOptional - 1 end - end - break - end - end - end - end - end - return copied, state -end - return RequiredImports diff --git a/tests/launcher_mods_install_zip_test.lua b/tests/launcher_mods_install_zip_test.lua index fc73a186..2ef9dd51 100644 --- a/tests/launcher_mods_install_zip_test.lua +++ b/tests/launcher_mods_install_zip_test.lua @@ -17,6 +17,7 @@ local ARCHIVE = { local files, dirs, arch = {}, {}, {} local fileDataMounts, pathMounts, stagedTemps = 0, 0, {} local stagedEver = false +local failWriteOnce local function resetFs() for k in pairs(files) do files[k] = nil end @@ -25,6 +26,7 @@ local function resetFs() fileDataMounts, pathMounts = 0, 0 stagedTemps = {} stagedEver = false + failWriteOnce = nil end local function dirChild(key, name) @@ -45,6 +47,10 @@ end local vfs = {} function vfs.write(name, data) + if failWriteOnce == name then + failWriteOnce = nil + return nil, "simulated write failure" + end files[name] = data if name:match("^mod_import_") then stagedTemps[name] = true @@ -232,6 +238,33 @@ eq(files["mods/" .. MOD_ID .. "/baseroms/stadium2.z64"], "user-owned-rom", check(files["mods/OldFolder/baseroms/stadium2.z64"] == nil, "the shadow mod tree is still removed after preservation") +-- A preservation write failure keeps recovery bytes outside mods/, where +-- discovery cannot mistake a baseroms-only directory for an installed mod. +resetFs() +files["mods/OldFolder/manifest.json"] = + ('{"id":"%s","name":"Old Copy","version":"0.9.0","entry":"main.lua"}') + :format(MOD_ID) +files["mods/OldFolder/main.lua"] = "return function() end\n" +files["mods/OldFolder/baseroms/stadium2.z64"] = "user-owned-rom" +files["imports/mods/preserve-fail.zip"] = "PK\3\4update" +failWriteOnce = "mods/" .. MOD_ID .. "/baseroms/stadium2.z64" +ok, err = LauncherMods.installZip("imports/mods/preserve-fail.zip", + { replace = true, expectId = MOD_ID }) +check(not ok, "preservation failure rejects the update") +check(files["mods/" .. MOD_ID .. "/manifest.json"] == nil, + "preservation failure leaves no manifest-less tree under mods") +eq(files["imports/baseroms-recovery/" .. MOD_ID .. "/stadium2.z64"], + "user-owned-rom", "preservation failure stages recovery outside mods") + +files["imports/mods/preserve-retry.zip"] = "PK\3\4update" +ok, err = LauncherMods.installZip("imports/mods/preserve-retry.zip", + { replace = true, expectId = MOD_ID }) +check(ok == true, "retry restores staged baseroms (" .. tostring(err) .. ")") +eq(files["mods/" .. MOD_ID .. "/baseroms/stadium2.z64"], "user-owned-rom", + "retry restores the recovered baserom into the installed mod") +check(files["imports/baseroms-recovery/" .. MOD_ID .. "/stadium2.z64"] == nil, + "successful retry clears baserom recovery debris") + -- #834: a manifest-less mods/ tree (interrupted copy debris) must not -- block a plain re-import as "already installed" resetFs() diff --git a/tests/mod_required_imports_tests.lua b/tests/mod_required_imports_tests.lua index 2d068508..271465b5 100644 --- a/tests/mod_required_imports_tests.lua +++ b/tests/mod_required_imports_tests.lua @@ -17,13 +17,17 @@ local manifest = Manifest.validate({ id = "stadium_fx", name = "Stadium FX", version = "1.0.0", entry = "main.lua", required_imports = { { id = "stadium2", name = "Stadium 2", file = "stadium2.z64", - format = "n64", md5 = { DIGEST, DIGEST:upper() } }, + description = "USA dump", format = "n64", size = 8, + md5 = { DIGEST, DIGEST:upper() } }, }, }, "mods/stadium_fx") eq(#manifest.required_imports, 1, "required import parses") eq(#manifest.required_imports[1].md5, 1, "accepted MD5 values normalize and dedupe") eq(manifest.required_imports[1].md5[1], DIGEST, "MD5 is lowercase") +eq(manifest.required_imports[1].description, "USA dump", + "import description is preserved for the picker UI") +eq(manifest.required_imports[1].size, 8, "exact import size parses") local optionalManifest = Manifest.validate({ id = "optional_fx", name = "Optional FX", version = "1.0.0", entry = "main.lua", @@ -49,6 +53,15 @@ check(not pcall(Manifest.validate, { id = "bad", name = "Bad", version = "1", entry = "main.lua", required_imports = { { id = "rom", file = "rom.z64", md5 = "short" } }, }), "malformed MD5 is refused") +check(not pcall(Manifest.validate, { + id = "bad", name = "Bad", version = "1", entry = "main.lua", + required_imports = { { id = "rom", file = ".rom.removed", md5 = DIGEST } }, +}), "hidden import filenames cannot collide with engine metadata") +check(not pcall(Manifest.validate, { + id = "bad", name = "Bad", version = "1", entry = "main.lua", + required_imports = { { id = "rom", file = "rom.bin", md5 = DIGEST, + max_size = RequiredImports.MAX_BYTES + 1 } }, +}), "manifest import sizes cannot exceed the hard limit") local canonical = "\128\55\18\64ABCD" local v64 = "\55\128\64\18BADC" @@ -79,17 +92,14 @@ local target = Manifest.validate({ { id = "same_rom", file = "source.z64", format = "n64", md5 = DIGEST }, }, }, "mods/other_fx") -local copied = RequiredImports.reconcile({ manifest, target }, love.filesystem, fakeHash) -eq(#copied, 1, "matching installed import is reused") -eq(love.filesystem.read("mods/other_fx/baseroms/source.z64"), canonical, - "reuse creates a private per-mod copy") +local targetRows, targetMissing = RequiredImports.inspect(target, + love.filesystem, fakeHash) +eq(targetMissing, 1, "matching hashes do not silently share another mod's import") +check(not targetRows[1].present, + "a mod needs its own explicit player-selected file") check(RequiredImports.remove(target, "same_rom"), "a required import can be removed") eq(love.filesystem.read("mods/other_fx/baseroms/source.z64"), nil, "remove deletes this mod's private copy") -local copiedAfterRemove = RequiredImports.reconcile({ manifest, target }, - love.filesystem, fakeHash) -eq(#copiedAfterRemove, 0, - "an explicit removal is not immediately undone by automatic reuse") check(RequiredImports.importData(target, "same_rom", canonical, { hash = fakeHash }), "choosing the file again clears the removal decision") @@ -103,16 +113,69 @@ local legacyTarget = Manifest.validate({ }, }, "mods/legacy_user") love.filesystem.write("mods/legacy/baseroms/manually-imported.v64", v64) -local legacyCopied = RequiredImports.reconcile({ legacy, legacyTarget }, +local legacyRows, legacyMissing = RequiredImports.inspect(legacyTarget, love.filesystem, fakeHash) -eq(#legacyCopied, 1, "an undeclared legacy baserom can satisfy a new declaration") -eq(love.filesystem.read("mods/legacy_user/baseroms/legacy-source.z64"), canonical, - "legacy reuse still stores canonical bytes") +eq(legacyMissing, 1, "undeclared files in another mod are never indexed") +check(not legacyRows[1].present, "legacy baseroms remain private to their mod") + +local capped = Manifest.validate({ + id = "capped", name = "Capped", version = "1.0.0", entry = "main.lua", + required_imports = { + { id = "small", file = "small.bin", md5 = DIGEST, max_size = 4 }, + }, +}, "mods/capped") +local tooLarge, sizeWhy = RequiredImports.validateData( + capped.required_imports[1], "12345", fakeHash) +eq(tooLarge, nil, "per-import size cap rejects before hashing") +check(tostring(sizeWhy):find("too large", 1, true) ~= nil, + "size rejection explains the limit") + +-- A successful validation writes an engine receipt. Matching size + modtime +-- lets later launcher refreshes avoid reading and hashing the ROM again. +local cacheFiles = { + ["mods/cache/baseroms/source.z64"] = canonical, +} +local dataReads = 0 +local cacheModtime = 123 +local cacheFs = { + getInfo = function(path, kind) + local data = cacheFiles[path] + if data then return { type = "file", size = #data, modtime = cacheModtime } end + return nil + end, + read = function(path) + if path == "mods/cache/baseroms/source.z64" then dataReads = dataReads + 1 end + return cacheFiles[path] + end, + write = function(path, data) cacheFiles[path] = data return true end, + remove = function(path) cacheFiles[path] = nil return true end, +} +local cacheManifest = Manifest.validate({ + id = "cache", name = "Cache", version = "1.0.0", entry = "main.lua", + required_imports = { + { id = "source", file = "source.z64", format = "n64", size = 8, + md5 = DIGEST }, + }, +}, "mods/cache") +local cacheRows = RequiredImports.inspect(cacheManifest, cacheFs, fakeHash) +check(cacheRows[1].present, "initial cached import validation succeeds") +cacheRows = RequiredImports.inspect(cacheManifest, cacheFs, function() + error("unchanged cached import should not be hashed again") +end) +check(cacheRows[1].present, "validation receipt satisfies the next refresh") +eq(dataReads, 1, "unchanged imported ROM is read only once") +cacheFiles["mods/cache/baseroms/source.z64"] = "BADBYTES" +cacheModtime = 124 +cacheRows = RequiredImports.inspect(cacheManifest, cacheFs, fakeHash) +check(not cacheRows[1].present, "changed imported ROM bypasses a stale receipt") +eq(dataReads, 2, "changed imported ROM is read again") +eq(cacheFiles[RequiredImports.receiptPath(cacheManifest, cacheManifest.required_imports[1])], + nil, "stale validation receipt is removed") local rejected, why = RequiredImports.importData(target, "same_rom", "wrong", { hash = fakeHash }) eq(rejected, nil, "mismatched selection is rejected") -check(tostring(why):find("Nintendo 64", 1, true) ~= nil, +check(tostring(why):find("N64 ROM (.z64/.v64/.n64)", 1, true) ~= nil, "normalization failure explains the selected format") love.filesystem.write("mods/launcher_needs/manifest.json", ([[{ diff --git a/tests/rom_importer_android_mod_pick_test.lua b/tests/rom_importer_android_mod_pick_test.lua index f863687e..b958e75b 100644 --- a/tests/rom_importer_android_mod_pick_test.lua +++ b/tests/rom_importer_android_mod_pick_test.lua @@ -135,6 +135,24 @@ check(ri.requiredImportNotice.text:find("MD5 mismatch", 1, true) ~= nil, check(ri.modNotice == nil, "required import rejection is not hidden in the general Mods notice") +-- Reported size is checked before the selected file is read into Lua. +local savedGetInfo = love.filesystem.getInfo +love.filesystem.getInfo = function(name, kind) + if name == "oversized_required.bin" then + return { type = "file", size = 10 } + end + return savedGetInfo(name, kind) +end +ri.mods[1].manifest.required_imports[1].max_size = 4 +ri._importRequiredSource = RomImporter._importRequiredSource +ri._importRequiredData = function(self) self._oversizedWasRead = true end +ri:_importRequiredSource("needs_source", "source", "oversized_required.bin") +check(not ri._oversizedWasRead, "oversized required file is rejected before import") +check(ri.requiredImportNotice.text:find("too large", 1, true) ~= nil, + "oversized required file reports its size error in the modal") +ri.mods[1].manifest.required_imports[1].max_size = nil +love.filesystem.getInfo = savedGetInfo + ri.nativePicker = true ri._importRequiredSource = function(self, modId, importId, source) self._requiredImported = { modId = modId, importId = importId, source = source }