• Shopkeepers version: 2.4.1
• Spigot version: git-Paper-470 (MC: 1.13.2) (Implementing API version 1.13.2-R0.1-SNAPSHOT)

Using the following permissions.yml setup...

default:
  default: true
  children:
    shopkeeper.player: false
    shopkeeper.player.sell: false
    shopkeeper.player.buy: false
    shopkeeper.player.trade: true
    shopkeeper.player.book: false

...non-OP players are able to create sell and trade shops, instead of only being able to create a trade shop:

Try to also remove the legacy permission 'shopkeeper.player.normal'. This is basically an alias for the sell-permission, and only still exists for backwards compatibility reasons.

# Legacy permissions:
shopkeeper.player.normal:
    description: Create and modify selling player shopkeepers
    default: true
    children:
        shopkeeper.player.sell: true

Edit: I can probably do some cleanup and remove those old legacy permissions for the next update.

Would it be possible for you to add a config option to always ignore that permission?

I don't think it's an issue with shopkeepers, nor with any specific permission node.. I was already able to reproduce this strange behavior with the other permission nodes as well (trade, buy, book, etc.).

So my recommendation currently is to use a permissions plugin instead for the time being (ex. PermissionsEx).

Permission plugins do too much. I only need to give and remove some permissions by default.

Well, I will keep looking. But right now I assume this is an bukkit issue. And since it isn't reliable reproducible this might be difficulty to debug..

Alternatively you could also modify the plugin.yml located inside the Shopkeepers.jar directly, and adjust the defaults there.
(Between, I just tried, and simply removing this legacy permission node does not fix this issue.)

You don't need to remove the legacy permission node, but a config setting to maybe disable the whole currency thing would be nice. It's not like players shouldn't be allowed to create shops, but I find emeralds are useless, even as a currency, and I don't want to enforce a server currency item, so trade shops are the best option.

default:
  default: true
  children:
    shopkeeper.player: false
    shopkeeper.player.normal: false
    shopkeeper.player.sell: false
    shopkeeper.player.buy: false
    shopkeeper.player.trade: true
    shopkeeper.player.book: false
    shopkeeper.hire: false

Still the same issue.

It seems like it cannot cope with the compound-permission shopkeeper.player somehow. Also the order seems to have an effect.
This seems to work though:

default:
  default: true
  children:
    shopkeeper.hire: false
    shopkeeper.player.buy: false
    shopkeeper.player.book: false
    shopkeeper.player.normal: false
    shopkeeper.player.sell: false
    shopkeeper.player.trade: true

Edit: I am not actually sure, but it seems to work sometimes and sometimes it doesn't.. Maybe this is some bukkit issue. I will have to further debug this at some point..

With your permission configuration:

Yeah, it seems to behave quite oddly: With the above permissions.yml and fresh server restarts, I was already able to sometimes get it working, and not working in other cases.. Same after reloads: Sometimes it works, and other times it doesn't.
I am pretty sure there is some underlying bukkit/spigot issue..

Edit: At some points it even applies some of the configured permissions and ignores others..

I think I have tracked down the underlying issue. You are currently not able to reliably override plugin-defined default permission values via the permissions.yml since the order in which those permissions get evaluated is arbitrary and can dynamically change. So sometimes your custom defined default permission values will successfully override the plugin-defined defaults, while at other times the plugin-defined default values will override your custom default values.

I have created a PR over at Spigot which should be able to resolve this issue at least in the common case (if plugin's aren't dynamically registering permissions): https://hub.spigotmc.org/stash/projects/SPIGOT/repos/bukkit/pull-requests/407/overview

Spigot has included the proposed changes. Can you retry on the latest version of Spigot and verify that this has worked as intended?

I'm using Paper. I'll have to wait for them to pull upstream.

Before the fix: Permissions from the permissions.yml and from plugins were combined in random order. The permissions that are last in that order are able to override the permissions that came before. This means that sometimes the plugin permissions would be in effect, and sometimes the permissions from the permissions.yml.

After the fix on spigot: The permissions.yml will get loaded after plugins and Bukkit will keep them in that order, so that they can override plugin permissions consistently.

Before the fix on paper: They are by default loading the permissions.yml before plugins (there is the above mentioned setting for this). I assume this is so that certain plugins can see those custom permissions and somehow treat them during startup. But if you don't have a plugin depending on this (eg. a permission plugin which depends on this), you should be able to safely disable this setting.

After the fix on paper (assuming everything else stays the same): If you don't change the above mentioned setting, the permissions.yml will get still be loaded before plugin permissions, meaning that they will not be able to override the plugin permissions.

If you are using Paper, you will probably also have to set the 'settings.load-permissions-yml-before-plugins' setting to 'false' (once they have pulled this change) for this fix to work.

I thought permissions.yml would override plugins by default.