[Bug]: Minecraft eggs should not force enable-query=true in server.properties, but should set query.port

[bcat]

Panel Version

1.11.7

Wings Version

1.11.13

Service

Minecraft

Modified

No, I did not modify the egg

Expected Behavior

I'm using the Fabric egg, but various other Minecraft eggs seem to have the same issue.

I expect to be able to disable Minecraft's query service for privacy reasons. Moreover, if I choose to leave the query service enabled, I expect it use the port number from the server's primary allocation. (The query service listens on UDP, whereas Minecraft itself listens on TCP.)

Actual Behavior

The various indicated Minecraft eggs are configured to force enable-query=true in the server.properties file every time the server is started. This prevents the egg's user from setting enable-query=false manually since it will be overwritten on every server startup. Essentially, this forces the query service to be enabled, which people may expose information about the server that's not intended (e.g., to sites like https://mcsrvstat.us/, as well as folks who run bots scanning for servers).

Additionally, if I did want the query service enabled, it still wouldn't work for me since the egg config does not set query.port to the port allocated in the panel, but rather leaves it the default (25565). Since I run Minecraft on a non-default port number, the query port won't actually be forwarded into the container if I did want it.

Steps To Reproduce

When trying to disable the query service:

1. Install any of the various Minecraft server eggs that forces enable-query=true.
2. Manually edit server.properties to set enable-query=false and restart the server.
3. Observe that enable-query has been set back to true.

When trying to use the query service with a non-default port number:

1. Install any of the various Minecraft server eggs mentioned above
2. Assign the server a port allocation that's not the default (25565).
3. Observe that the query service is configured to listen on the default port rather than the allocated port, so even those it's forced on, it isn't actually accessible outside the container.

---

IMO, it makes sense to change the various egg configs that look like

{
    "server.properties": {
        "parser": "properties",
        "find": {
            "server-ip": "0.0.0.0",
            "enable-query": "true",
            "server-port": "{{server.build.default.port}}"
        }
    }
}

to something like this instead:

{
    "server.properties": {
        "parser": "properties",
        "find": {
            "server-ip": "0.0.0.0",
            "server-port": "{{server.build.default.port}}",
            "query.port": "{{server.build.default.port}}"
        }
    }
}

That way, the server owner can enable or disable the query service as they see fit, but if they do want it enabled, the query service will automatically listen on the correct (allocated) port.

Install logs

N/A

[bcat]

Interestingly, a number of the eggs already set query.port as I suggested. But not all of them (e.g., the Fabric egg I'm using).

Unless there's a strong reason for some of the eggs to behave differently, I think it'd be less confusing (and safer) for them to all consistently set query.port, and not set enable-query. WDYT?

Note that I don't mind sending a PR for this change, but I do think consistency is important and I wanted to get a sanity check before editing a bunch of eggs en masse.

[QuintenQVD0]

I am not chaning this logic. It used to be that the parser would set the query port only not the enabled. If that is the case for some egg feel free to PR it but that is a liite wasted time. And that listening on the default port is because thats mc default config gen value and if the parser does not change it then it stays at that.

[bcat]

Right, so I agree setting the query port automatically may not be super valuable (and folks can manually edit the eggs to do so if they want). That seemed like more of a "nice to have" since I was suggesting some egg config changes anyway. (It's mostly that a number of the MC eggs already set query.port, so making them all do it would improve the consistency from one egg to another.)

But the bigger issue I see is the 14 eggs that force enable-query=true. This prevents a server owner from disabling the query service unless they edit the egg config to remove that setting. (Otherwise, the relevant bit of server.properties gets rewritten on every server startup to re-enable query.)

I guess if this was intentional for some reason (like Pterodactyl or Pelican itself using the query port to monitor server status), I'd feel differently, but as far as I can tell that's not the case. Maybe I'm just missing something, though?

[QuintenQVD0]

Right, so I agree setting the query port automatically may not be super valuable (and folks can manually edit the eggs to do so if they want). That seemed like more of a "nice to have" since I was suggesting some egg config changes anyway. (It's mostly that a number of the MC eggs already set query.port, so making them all do it would improve the consistency from one egg to another.)

But the bigger issue I see is the 14 eggs that force enable-query=true. This prevents a server owner from disabling the query service unless they edit the egg config to remove that setting. (Otherwise, the relevant bit of server.properties gets rewritten on every server startup to re-enable query.)

I guess if this was intentional for some reason (like Pterodactyl or Pelican itself using the query port to monitor server status), I'd feel differently, but as far as I can tell that's not the case. Maybe I'm just missing something, though?

We indeed do not use the query port for anything so there should be no reason that we force set this go true. So I fo agree that we would change that in the future.

[bcat]

Okay, I can send a PR for that in a bit, then.

[myxxmikeyxx]

@bcat did you ever get to making that pull request?

[bcat]

@bcat did you ever get to making that pull request?

No, sorry, it got lost in the shuffle of things.

FWIW, the egg I'm using now (the NeoForge one) is already set up how I was thinking (forces query.port, but doesn't force enable-query). This issue may be obsolete; I'll take a quick look at the other eggs....

[bcat]

Ah, it looks like there's still inconsistency across some of the MC eggs. Since the NeoForge one I'm using now does the "right thing", this is honestly pretty low priority for me, but I do still think it would make sense to change them. (Sorry to be lazy and not just do it; it's just crazy busy times, as it goes.)