Skip to content

Prevent loading of mods with invalid names#16904

Open
compmstr wants to merge 9 commits intoluanti-org:masterfrom
compmstr:mod-name-filter-tweak
Open

Prevent loading of mods with invalid names#16904
compmstr wants to merge 9 commits intoluanti-org:masterfrom
compmstr:mod-name-filter-tweak

Conversation

@compmstr
Copy link

@compmstr compmstr commented Feb 3, 2026

Logs message to warningstream

Add compact, short information about your PR for easier understanding:

  • Goal of the PR:
    • Stop loading mod, and log warning when an invalid name is detected in mod.conf
  • How does the PR work?
    • Updates parseModContents to check for invalid name when loading. Logs and skips mods containing invalid characters.
  • Does it resolve any reported issue?
  • Does this relate to a goal in the roadmap?
    • This is a small bug fix that was listed as a good starter fix.
  • If you have used an LLM/AI to help with code or assets, you must disclose this.
    • I have not used any LLM/AI to develop this

I realize there's already an open PR for this, but it has been inactive for a couple of weeks at this point.

To do

This PR is Ready for Review.

How to test

As documented in the issue, set up a mod with invalid characters in the name field.

ex:

name = """
haha ;]
oops
"""
description = "Mod to test bug that's being fixed"
depends = first_mod

Prior to this fix, the mod would load, and show up oddly in the mod list.

Afterwards, there is a WARN log, and the mod does not get loaded/shown.

@sfan5 sfan5 added the Bugfix 🐛 PRs that fix a bug label Feb 3, 2026
@SmallJoker
Copy link
Member

Alternative PR: #16719

if (info.exists("name")) {
spec.name = info.get("name");
spec.is_name_explicit = true;
if (!string_allowed(spec.name, MODNAME_ALLOWED_CHARS)) {
Copy link
Member

@SmallJoker SmallJoker Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When starting Luanti in the "Start game" tab, the following warnings are logged. When I click "Configure mods", the same bunch of warnings are logged - again.

Error loading mod "xxx/games/VoxeLibre/mods/PLAYER": Mod does not follow naming conventions: Only characters [a-z0-9_] are allowed.
Error loading mod "xxx/mods/worldedit-modpack/bonkers": Mod does not follow naming conventions: Only characters [a-z0-9_] are allowed.
Error loading mod "xxx/mods/worldedit-modpack": Mod does not follow naming conventions: Only characters [a-z0-9_] are allowed.
Error loading mod "xxx/mods/skyblock": Mod does not fo	llow naming conventions: Only characters [a-z0-9_] are allowed.
Error loading mod "xxx/mods/nested_modpack/modpack_a1": Mod does not follow naming conventions: Only characters [a-z0-9_] are allowed.
Error loading mod "xxx/mods/nested_modpack/modpack_b2": Mod does not follow naming conventions: Only characters [a-z0-9_] are allowed.
Error loading mod "xxx/mods/nested_modpack": Mod does not follow naming conventions: Only characters [a-z0-9_] are allowed.
  1. The warnings should only logged once per mod(pack) to avoid spam.
  2. From what I could(n't find), modpacks seem to have no naming rules defined in lua_api.md
    • This needs documenting.
    • A-z_- or a-z_- might be the best retro-fit.
  3. Perhaps misleading PR title. Prevent loading of mods is done by mod.checkAndLog(); in ServerModManager::loadMods, where the server will not load the mods that do not obey the naming conventions.

EDIT: Whereas the naming is enforced upon server start-up, there is also lack of documentation on mod names in lua_api.md.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll notice that nothing else prints to the log in this function. This is by design - it would be too spammy in the main menu. You should just return false here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, should I just drop the logging altogether, or try to find somewhere to log once for mod debugging purposes?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I could add a message to deprecation_msgs, so that it gets logged out when other mod issues are.

I could also set up a new error_msgs that contains this message, as well as things like "Mod ... missing init.lua" (the other case where we skip loading a log). I could log it from within checkAndLog the same way deprecation messages are.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look at how the missing init.lua is validated because it's the same style of issue

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got bit by the silent failure of the mod from a missing init.lua when I first tried to test this PR. Ideally we could log at least once whenever there's an error loading the mod.

I'll take another pass at this, see if I can get these to log out inside checkAndLog, since that's less spammy.

Update allowed mod name characters to include A-Z (upper-case)
ModSpec::mod_load_error_msgs holds the set of errors (per file)

When `checkAndLog` is called, we also log mod load errors

Logging, ignoring, and erroring are all configured by a new setting (defaulted
to log)
@compmstr
Copy link
Author

compmstr commented Feb 3, 2026

I set up a static mod_load_error_msgs member to keep track of per-path module load errors.
When we call checkAndLog on modules, we'll also call logModLoadErrors, which will either ignore, log, or error out on module load errors. There's a new configuration, mod_error_handling that controls what we do with the error messages.
I added a flag so that we only log those errors one time, instead of logging from each module.

Error (when mod_error_handling is set to error):
image

Logs:

2026-02-03 15:18:25: ERROR[Main]: ModError: Mod at /home/corey/stuff/OpenSource/luanti/games/devtest/mods/bad_name2:
2026-02-03 15:18:25: ERROR[Main]: 	Mod does not contain 'init.lua
2026-02-03 15:18:25: ERROR[Main]: Mod at /home/corey/stuff/OpenSource/luanti/mods/bad_name:
2026-02-03 15:18:25: ERROR[Main]: 	Mod does not follow naming conventionsOnly characters [a-zA-Z0-9_] are allowed.
2026-02-03 15:18:25: ERROR[Main]: Check debug.txt for details.
2026-02-03 15:18:25: ACTION[ServerStop]: Server: Shutting down
2026-02-03 15:19:06: WARNING[ServerStart]: Mod at /home/corey/stuff/OpenSource/luanti/games/devtest/mods/bad_name2:
2026-02-03 15:19:06: WARNING[ServerStart]: 	Mod does not contain 'init.lua
2026-02-03 15:19:06: WARNING[ServerStart]: Mod at /home/corey/stuff/OpenSource/luanti/mods/bad_name:
2026-02-03 15:19:06: WARNING[ServerStart]: 	Mod does not follow naming conventionsOnly characters [a-zA-Z0-9_] are allowed.

@compmstr
Copy link
Author

compmstr commented Feb 8, 2026

Would this be cleaner if I set up something like a ModErrors class that keeps track of these errors? Rather than include it in the existing mods.(h|cpp)? Or is this approach with the static store of messages a non-starter?

Since the errors are explicily for mods that did not get loaded, it made more
  sense to have this be separate from ModSpec.
@compmstr
Copy link
Author

I moved this mod error handling code into a new ModErrors class, since it's explicitly for errors that stop a mod from loading. So having it stored in ModSpec made less sense, imo.

@sfan5 sfan5 added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Feb 17, 2026
Move back to original valid name characters
Fix error message to reflect actual allowed characters.
@compmstr compmstr requested review from appgurueu and sfence February 17, 2026 13:32
@sfan5 sfan5 removed their request for review February 19, 2026 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Action / change needed Code still needs changes (PR) / more information requested (Issues) Bugfix 🐛 PRs that fix a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Insufficient validation of mod.conf name

6 participants