minetest-mods / skinsdb

Player skin mod, supporting unified_inventory, sfinv and smart_inventory
27 stars 41 forks source link

Mod should not download files directly to its own directory -- use world dir instead #31

Open VanessaE opened 4 years ago

VanessaE commented 4 years ago

Mods must not create or write files inside their own mod directory -- only inside the world directory.

Saving files inside the mod's own directory is incompatible with Mod Security and incompatible with any workflow that deletes and rebuilds a server's mods tree periodically (as my servers do every morning; the skins would normally be copied-in from another location after the tree is built).

bell07 commented 4 years ago

Do you mean the skins_updater? It is just a helper tool for server admin to download skins from http://minetest.fensta.bplaced.net like the bash /python/C# scripts did it before. You are not forced to use it, and you are not forced to set skinsdb to the trusted_mods. I do not use them too, synchronize my skins using syncthing.

What should be changed from your point of view?

VanessaE commented 4 years ago

I would suggest moving the downloader into a separate mod, because you are forced to add skinsdb to secure-trusted to get the downloader to work.

That way, skinsdb itself would run with Mod Security intact, and the downloader mod can be as insecure as it wants. :stuck_out_tongue:

Plus, whether or not it's a separate mod (if skinsdb soft-depends on it), the downloader could directly save the meta and downloaded skins in the world directory (in their own directory, of course) when that command is run, and at startup, it could copy (or perhaps on supported systems, symbolically-link using some os. function) them to somewhere skinsdb and Minetest can find them. I checked with a few Minetest devs, this method should work -- the server doesn't gather media until after mod init is done.

SmallJoker commented 4 years ago

Saving files inside the mod's own directory is incompatible with Mod Security

No, it is compatible. Mods may write to their own directories as well.

because you ARE forced to add skinsdb to secure-trusted, or Minetest stops it dead because of the insecure environment request at

Is your clone up to date? Minetest starts just fine with all restrictions on:

secure.enable_security = true
secure.trusted_mods =
secure.http_mods =

You do not need to disable the entire security stuff, following settings will work:


secure.enable_security = true
secure.trusted_mods = skinsdb
secure.http_mods = skinsdb
`` 
VanessaE commented 4 years ago

So, I was wrong about the error, I misremembered.. It only happens if I try to run the downloader without having configured as above, and doesn't kill the server. I guess a separate mod is not needed.

However, my original idea still stands: do not download images/meta directly to the mod folder, download them to the world folder, then copy that data from there to skinsdb's textures folder at the next reboot (possibly checking for the presence of each file at the target before copying). The mod tree is not guaranteed to persist between reboots (and if needed, the server's management scripts can populate skinsdb's textures/meta from the files in the world folder before the day's first reboot)

mio-19 commented 4 years ago

https://github.com/minetest-mods/skinsdb/pull/43 will fix this