Deny instead of crashing when a control permission check throws (#1033)

A permission function runs during ingress permission checking, before
the request body is validated. UserBulkImportRequest.permission read the
raw, unvalidated message.data and threw a bare Error for an unknown
importType. That error is not a PermissionError, so _processMessage
rethrew it to the connector "message" listener, which emitted "error" on
the connector. Controller-side connectors have no "error" listener, so
Node rethrew and handleUnhandledErrors exited the process.

Repro: a control client with client-side validateSent disabled sends
UserBulkImportRequest with importType "foo"; the client gets a normal
RequestError response and the controller then logs a fatal uncaught
exception and exits.

Fix both layers: _processMessage now logs and drops an unexpected error
from the permission check (control connections only, where a response
was already sent) instead of rethrowing, and UserBulkImportRequest
throws a PermissionError for an unknown importType so it is denied
cleanly. A genuine PermissionError still denies silently as before.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

test/lib/data/messages_user.test.js

@@ -0,0 +1,27 @@
+import assert from "node:assert/strict";
+import * as lib from "@clusterio/lib";
+
+describe("lib/data/messages_user", function() {
+	describe("UserBulkImportRequest", function() {
+		// A user that grants every permission, so only the importType switch
+		// can reject the request.
+		function grantAllUser() {
+			const checked = [];
+			return { checked, checkPermission(permission) { checked.push(permission); } };
+		}
+
+		it("should deny an unknown importType with a PermissionError", function() {
+			const user = grantAllUser();
+			assert.throws(
+				() => lib.UserBulkImportRequest.permission(user, { data: { importType: "foo", users: [] } }),
+				lib.PermissionError,
+			);
+		});
+
+		it("should check permissions for a known importType", function() {
+			const user = grantAllUser();
+			lib.UserBulkImportRequest.permission(user, { data: { importType: "admins", users: [] } });
+			assert.deepEqual(user.checked, ["core.user.bulk_import", "core.user.set_admin"]);
+		});
+	});
+});

test/lib/link/link.js

@@ -539,6 +539,51 @@ describe("lib/link/link", function() {
 				);
 			});
 
+			describe("permission checking", function() {
+				// A control connection has connector.dst.type === control, which is
+				// what gates permission checking, see link.js _processMessage.
+				function controlSetup() {
+					const connector = new mock.MockConnector(dst, src);
+					const link = new lib.Link(connector);
+					const message = new lib.MessageRequest(1, src, dst, "SimpleRequest");
+					return { connector, link, message };
+				}
+
+				it("should silently deny on PermissionError from the permission check", function() {
+					const { connector, link, message } = controlSetup();
+					let handled = false;
+					let errored = false;
+					connector.on("error", () => { errored = true; });
+					link.handle(SimpleRequest, async () => { handled = true; });
+					link.validatePermission = () => { throw new lib.PermissionError("Denied"); };
+					connector.emit("message", message);
+					assert(!handled, "handler ran despite denial");
+					assert(!errored, "connector emitted error on denial");
+					assert.deepEqual(connector.sentMessages, [], "denial sent a message");
+				});
+
+				it("should not crash when the permission check throws a non-PermissionError", function() {
+					const { connector, link, message } = controlSetup();
+					let handled = false;
+					let errored = false;
+					const logged = [];
+					const originalError = lib.logger.error;
+					lib.logger.error = msg => { logged.push(msg); };
+					connector.on("error", () => { errored = true; });
+					try {
+						link.handle(SimpleRequest, async () => { handled = true; });
+						link.validatePermission = () => { throwSimple("check is broken"); };
+						connector.emit("message", message);
+					} finally {
+						lib.logger.error = originalError;
+					}
+					assert(!errored, "non-PermissionError escaped to the connector error path");
+					assert(!handled, "handler ran after the permission check threw");
+					assert.equal(logged.length, 1);
+					assert(logged[0].startsWith("Unexpected error checking permission for SimpleRequest:\n"));
+				});
+			});
+
 			describe("Broadcast handling", function() {
 				// A broadcast is addressed to every link of the type it targets,
 				// so a link it reaches has to pass it on as well as handle it,