Merge remote-tracking branch 'upstream/master' into fix/stop-before-rcon-ready

This commit is contained in:
bbassie committed 2026-09-13 17:10:02 +00:00
commit 0273558edc
16 files changed
+410 -27

No files matched your search

+19 -4
View File
@@ -48,12 +48,16 @@ export default class ControlConnection extends BaseConnection {
this._version = registerData.version;
this.connector.on("connect", () => {
this.connector._socket!.clusterio_ignore_dump = Boolean(this.ws_dumper);
});
// A resume installs a fresh socket, so re-apply the ignore flag or the
// dumper's own frames get echoed back into it and recurse forever.
for (let event of ["connect", "resume"] as const) {
this.connector.on(event, () => {
this.connector._socket!.clusterio_ignore_dump = Boolean(this.ws_dumper);
});
}
this.connector.on("close", () => {
if (this.logTransport) {
logger.remove(this.logTransport);
this._controller.clusterLogger.remove(this.logTransport);
this.logTransport = null;
}
if (this.ws_dumper) {
@@ -288,6 +292,9 @@ export default class ControlConnection extends BaseConnection {
}
async handleInstanceCreateRequest(request: lib.InstanceCreateRequest) {
if ((request.config["instance.assigned_host"] ?? null) !== null) {
throw new lib.RequestError("instance.assigned_host must be set through the assign-host interface");
}
const instanceConfig = new lib.InstanceConfig("controller");
if (request.cloneFromId) {
const baseInstance = this._controller.instances.get(request.cloneFromId);
@@ -1070,6 +1077,11 @@ export default class ControlConnection extends BaseConnection {
}
this._controller.users.deleteUser(user);
for (let controlConnection of this._controller.wsServer.controlConnections.values()) {
if (controlConnection.user.id === user.id) {
controlConnection.connector.terminate();
}
}
if (user.isAdmin) {
this._controller.sendTo("allInstances", new lib.InstanceAdminlistUpdateEvent(name, false));
@@ -1325,6 +1337,9 @@ export default class ControlConnection extends BaseConnection {
}
async handleDebugDumpWsRequest(request: lib.DebugDumpWsRequest) {
if (this.ws_dumper) {
return; // Already dumping
}
this.ws_dumper = data => {
if (this.connector.connected) {
this.send(new lib.DebugWsMessageEvent(data.direction, data.content));
+11 -3
View File
@@ -208,9 +208,17 @@ export default class InstanceManager {
instance.config.set("instance.assigned_host", hostId);
if (newHostConnection) {
await newHostConnection.send(
new lib.InstanceAssignInternalRequest(instanceId, instance.config.toRemote("host")),
);
try {
await newHostConnection.send(
new lib.InstanceAssignInternalRequest(instanceId, instance.config.toRemote("host")),
);
} catch (err) {
// Leave unassigned so the assign can be retried
instance.config.set("instance.assigned_host", null);
instance.status = "unassigned";
this.records.set(instance);
throw err;
}
} else {
instance.status = "unassigned";
}
+3 -1
View File
@@ -119,7 +119,9 @@ export default class UserManager {
roles.add(defaultRoleId);
}
const user = new User(this.records, this._controllerRoles, 0, name, roles);
// Reject tokens issued before the user existed, e.g. for a previous user by the same name
const tokenValidAfter = Math.floor(Date.now() / 1000);
const user = new User(this.records, this._controllerRoles, tokenValidAfter, name, roles);
this.records.set(user);
return user;
}
+14 -4
View File
@@ -306,7 +306,12 @@ export default class Instance extends lib.Link {
return;
}
let name = /^([^ ]+)/.exec(parsed.message)![1];
// Guard against lines that classify as actions but lack a name
let nameMatch = /^([^ ]+)/.exec(parsed.message);
if (!nameMatch) {
return;
}
let name = nameMatch[1];
// Detect player leave and join
if (parsed.action === "JOIN") {
@@ -322,7 +327,9 @@ export default class Instance extends lib.Link {
// Detect banned and admin state change, whitelist changes are not logged
if (parsed.action === "BAN") {
const reason = /\. Reason: (.+)\.$/.exec(parsed.message)![1];
// Ban lines without a ". Reason: X." tail default to no reason
const reasonMatch = /\. Reason: (.+)\.$/.exec(parsed.message);
const reason = reasonMatch ? reasonMatch[1] : "";
this._recordUserUpdate(parsed.action, name, reason !== "unspecified" ? reason : "");
} else if (["UNBANNED", "PROMOTE", "DEMOTE"].includes(parsed.action)) {
this._recordUserUpdate(parsed.action as "UNBANNED" | "PROMOTE" | "DEMOTE", name);
@@ -348,8 +355,11 @@ export default class Instance extends lib.Link {
return;
}
const name = /^([^ ]+)/.exec(parsed.message)![1];
this._recordUserUpdate(parsed.action as "PROMOTE" | "DEMOTE", name);
const nameMatch = /^([^ ]+)/.exec(parsed.message);
if (!nameMatch) {
return;
}
this._recordUserUpdate(parsed.action as "PROMOTE" | "DEMOTE", nameMatch[1]);
});
}
+3 -1
View File
@@ -2,6 +2,7 @@ import { Type, Static } from "@sinclair/typebox";
import UserDetails, { IUser } from "./UserDetails.js";
import { StringEnum, jsonArray, plainJson } from "./composites.js";
import { MessageRequest } from "./messages_core.js";
import { PermissionError } from "../errors.js";
export class UserGetRequest {
declare ["constructor"]: typeof UserGetRequest;
@@ -276,8 +277,9 @@ export class UserBulkImportRequest {
user.checkPermission("core.user.set_whitelisted");
break;
default:
// Unvalidated input during permission check: deny, don't throw a bare Error
// @ts-expect-error Unreachable
throw new Error(`Unknown import / restore type: ${data.importType}`);
throw new PermissionError(`Unknown import / restore type: ${data.importType}`);
}
}
}
+11 -7
View File
@@ -196,16 +196,20 @@ export class Link {
let entry = this._validateMessage(message);
if (entry && this.connector.dst.type === libData.Address.control) {
let permissionMessage = message as libData.MessageRequest | libData.MessageEvent;
try {
this.validatePermission(
message as libData.MessageRequest | libData.MessageEvent,
entry
); // Somewhat hacky, defined in ControlConnection
this.validatePermission(permissionMessage, entry); // Somewhat hacky, defined in ControlConnection
} catch (err) {
if (err instanceof libErrors.PermissionError) {
return;
// A non-PermissionError means the permission check itself is
// broken. validatePermission (control connections only) already
// sent a response error, so log and drop instead of crashing.
if (!(err instanceof libErrors.PermissionError)) {
logger.error(
`Unexpected error checking permission for ${permissionMessage.name}:\n` +
`${(err as Error).stack ?? (err as Error).message}`
);
}
throw err;
return;
}
}
@@ -44,7 +44,7 @@ export default function LoadScenarioModal(props: LoadScenarioModalProps) {
}
if (values.mapSettings && values.mapSettings.trim()) {
try {
mapSettings = JSON.parse(values.mapGenSettings);
mapSettings = JSON.parse(values.mapSettings);
} catch (err: any) {
form.setFields([{ name: "mapSettings", errors: [err.message] }]);
return;
@@ -146,6 +146,13 @@ export default function UserViewPage() {
new lib.UserDeleteRequest(userName)
).then(() => {
navigate("/users");
}, err => {
if (err instanceof lib.SessionLost && userName === account.name) {
// Got kicked out after deleting our own account
navigate("/users");
return;
}
throw err;
}).catch(notifyErrorHandler("Error deleting user"));
}}
>
+172 -1
View File
@@ -1,7 +1,44 @@
import assert from "node:assert/strict";
import { ControlConnection } from "@clusterio/controller";
import events from "node:events";
import * as lib from "@clusterio/lib";
import { Controller, ControlConnection, ControllerHooks, InstanceManager } from "@clusterio/controller";
import { MockConnector, MockController, MockLogger } from "../mock.js";
const addr = lib.Address.fromShorthand;
describe("controller/src/ControlConnection", function() {
describe(".handleDebugDumpWsRequest()", function() {
function makeConnection() {
const controllerConfig = new lib.ControllerConfig("controller");
const connector = new lib.VirtualConnector(
lib.Address.fromShorthand("controller"),
lib.Address.fromShorthand({ controlId: 1 }),
);
connector._socket = {};
const controller = new Controller(lib.logger, [], controllerConfig);
const user = controller.users.getOrCreateUser("test");
return new ControlConnection({ version: "2.0.0" }, connector, controller, user, 1);
}
it("re-applies clusterio_ignore_dump to the socket on resume", async function() {
const connection = makeConnection();
await connection.handleDebugDumpWsRequest(new lib.DebugDumpWsRequest());
// A resume installs a fresh socket without the flag.
const resumedSocket = {};
connection.connector._socket = resumedSocket;
connection.connector.emit("resume");
assert.equal(resumedSocket.clusterio_ignore_dump, true);
});
it("leaves the flag off on resume without a dumper", function() {
const connection = makeConnection();
const resumedSocket = {};
connection.connector._socket = resumedSocket;
connection.connector.emit("resume");
assert.equal(resumedSocket.clusterio_ignore_dump, false);
});
});
describe(".handleControllerRestartRequest()", function() {
let mockController;
@@ -38,4 +75,138 @@ describe("controller/src/ControlConnection", function() {
assert.equal(mockController.stopped, false);
});
});
describe(".handleInstanceCreateRequest()", function() {
let mockController;
beforeEach(function() {
mockController = new MockController();
mockController.mockConfigEntries.set("controller.name", "Test");
mockController.instances = new InstanceManager(new lib.SubscribableDatastore(), mockController);
});
async function create(config, cloneFromId) {
await ControlConnection.prototype.handleInstanceCreateRequest.call(
{ _controller: mockController },
new lib.InstanceCreateRequest(config, cloneFromId),
);
}
it("rejects instance.assigned_host in the config", async function() {
await assert.rejects(
create({ "instance.id": 4001, "instance.name": "c1", "instance.assigned_host": 10 }),
new lib.RequestError("instance.assigned_host must be set through the assign-host interface"),
);
assert.equal(mockController.instances.has(4001), false);
});
it("creates an unassigned clone of an assigned instance", async function() {
await create({ "instance.id": 4001, "instance.name": "base" });
mockController.instances.getMutable(4001).config.set("instance.assigned_host", 10);
await create({ "instance.id": 4002, "instance.name": "clone", "instance.assigned_host": null }, 4001);
const clone = mockController.instances.get(4002);
assert.equal(clone.config.get("instance.name"), "clone");
assert.equal(clone.config.get("instance.assigned_host"), null);
});
});
describe("close cleanup", function() {
let connector;
let mockController;
let connection;
beforeEach(function() {
connector = new MockConnector(addr("controller"), addr({ controlId: 1 }));
connector._socket = {};
const transports = new Set();
mockController = {
hooks: new ControllerHooks(new MockLogger()),
router: null,
_registeredRequests: new Map(),
_fallbackedRequests: new Map(),
_registeredEvents: new Map(),
_snoopedEvents: new Map(),
subscriptions: { unsubscribeLink() {} },
clusterLogger: {
transports,
add(transport) { transports.add(transport); },
remove(transport) { transports.delete(transport); },
},
debugEvents: new events.EventEmitter(),
sendRequestToHostByInstanceId() {},
};
connection = new ControlConnection({ version: "test" }, connector, mockController, {}, 1);
});
it("removes the log transport from clusterLogger", async function() {
await connection.handleLogSetSubscriptionsRequest(
new lib.LogSetSubscriptionsRequest(true, false, [], [])
);
assert.equal(mockController.clusterLogger.transports.size, 1);
connector.emit("close");
assert.equal(mockController.clusterLogger.transports.size, 0);
assert.equal(connection.logTransport, null);
});
it("removes the debug dump listener after repeated requests", async function() {
await connection.handleDebugDumpWsRequest(new lib.DebugDumpWsRequest());
await connection.handleDebugDumpWsRequest(new lib.DebugDumpWsRequest());
assert.equal(mockController.debugEvents.listenerCount("message"), 1);
assert.equal(connector._socket.clusterio_ignore_dump, true);
connector.emit("close");
assert.equal(mockController.debugEvents.listenerCount("message"), 0);
});
});
describe(".handleUserDeleteRequest()", function() {
let mockController;
let nextId;
beforeEach(function() {
mockController = new MockController();
mockController.sendTo = () => {};
mockController.wsServer.controlConnections = new Map();
nextId = 1;
});
function connect(name) {
const connection = {
_controller: mockController,
user: mockController.users.getByName(name),
connector: {
terminated: false,
terminate() { this.terminated = true; },
},
};
mockController.wsServer.controlConnections.set(nextId, connection);
nextId += 1;
return connection;
}
async function deleteUser(connection, name) {
await ControlConnection.prototype.handleUserDeleteRequest.call(
connection, new lib.UserDeleteRequest(name)
);
}
it("terminates connections of the deleted user", async function() {
const admin = connect("test");
const player1 = connect("player");
const player2 = connect("player");
await deleteUser(admin, "player");
assert.equal(mockController.users.getByName("player"), undefined);
assert.equal(player1.connector.terminated, true);
assert.equal(player2.connector.terminated, true);
assert.equal(admin.connector.terminated, false);
});
it("terminates its own connection when deleting itself", async function() {
const player = connect("player");
await deleteUser(player, "player");
assert.equal(player.connector.terminated, true);
});
});
});
+39
View File
@@ -298,6 +298,45 @@ describe("controller/InstanceManager", function () {
assert(assignRequest instanceof lib.InstanceAssignInternalRequest);
assert.equal(assignRequest.instanceId, 1);
});
it("should leave instance unassigned if the new host fails the assign", async function () {
let unassignRequest;
controller.wsServer.hostConnections.set(5, {
send: async (request) => {
unassignRequest = request;
},
connector: { closing: false },
});
let failAssign = true;
let assignRequest;
controller.wsServer.hostConnections.set(6, {
send: async (request) => {
if (failAssign) {
throw new lib.RequestError("EACCES: permission denied");
}
assignRequest = request;
},
connector: { closing: false },
});
instance.config.set("instance.assigned_host", 5);
instance.status = "stopped";
await assert.rejects(
() => instances.assignInstance(1, 6),
{ message: "EACCES: permission denied" }
);
assert(unassignRequest instanceof lib.InstanceUnassignInternalRequest);
const stored = instances.records.get(1);
assert.equal(stored.config.get("instance.assigned_host"), null);
assert.equal(stored.status, "unassigned");
failAssign = false;
await instances.assignInstance(1, 6);
assert.equal(instance.config.get("instance.assigned_host"), 6);
assert(assignRequest instanceof lib.InstanceAssignInternalRequest);
assert.equal(assignRequest.instanceId, 1);
});
});
describe(".unassignInstance()", function () {
+17
View File
@@ -79,6 +79,23 @@ describe("controller/UserManager", function () {
const user = userManager.createUser("Eve");
assert(user.roleIds.has(1));
});
it("should set tokenValidAfter to the current time", function () {
const before = Math.floor(Date.now() / 1000);
const user = userManager.createUser("Mallory");
assert(user.tokenValidAfter >= before, "tokenValidAfter predates the call");
assert(user.tokenValidAfter <= Math.floor(Date.now() / 1000), "tokenValidAfter is in the future");
});
it("should invalidate tokens issued before a user of the same name was recreated", function () {
const user = userManager.createUser("Niaj");
const oldTokenIat = user.tokenValidAfter - 60;
userManager.deleteUser(user);
// WsServer.registerControl rejects the token when iat < tokenValidAfter
const recreated = userManager.createUser("Niaj");
assert(oldTokenIat < recreated.tokenValidAfter, "token from before the deletion is still valid");
});
});
describe(".getOrCreateUser()", function () {
+33
View File
@@ -328,4 +328,37 @@ describe("class Instance", function() {
}
});
});
describe("_watchServerLogActions()", function() {
beforeEach(function() {
instance.config.set("factorio.sync_banlist", "bidirectional");
instance._watchServerLogActions();
});
afterEach(function() {
clearInterval(instance._playerCheckInterval);
});
function emitAction(action, message) {
instance.server.emit("output", { type: "action", action, message }, "");
}
it("should record a normal JOIN action", function() {
emitAction("JOIN", "player joined the game");
assert(instance.playersOnline.has("player"), "player was not added");
});
it("should parse the reason of a normal BAN action", function() {
emitAction("BAN", "player was banned by <server>. Reason: griefing.");
assert.deepEqual(
connector.sentMessages[0],
new lib.MessageEvent(
1, src, addr("allInstances"), "InstanceBanlistUpdateEvent",
new lib.InstanceBanlistUpdateEvent("player", true, "griefing"),
),
);
});
it("should not throw on a BAN action without a reason tail", function() {
assert.doesNotThrow(() => emitAction("BAN", "evil"));
});
it("should not throw on an action whose message starts with a space", function() {
assert.doesNotThrow(() => emitAction("JOIN", " "));
});
});
});
+3 -1
View File
@@ -191,7 +191,8 @@ let haveFactorioInstall;
const baseHostConfig = loadJSON("config-host.json");
export let url = "https://localhost:4443/";
export let controlToken = jwt.sign({ aud: "user", user: "test" }, Buffer.from("TestSecretDoNotUse", "base64"));
// Signed in the root before hook, as the user has to exist before the token is issued
export let controlToken;
export let instancesDir = path.join("temp", "test", "instances");
export let modsDir = path.join("temp", "test", "mods");
export let databaseDir = path.join("temp", "test", "database");
@@ -373,6 +374,7 @@ before(async function() {
console.log("Bootstrapping");
await execController("bootstrap create-admin test");
await execController("bootstrap create-ctl-config test");
controlToken = jwt.sign({ aud: "user", user: "test" }, Buffer.from("TestSecretDoNotUse", "base64"));
controllerProcess = await spawnNode("controller:", "../../packages/controller run", /Started controller/);
+5 -4
View File
@@ -2,10 +2,11 @@ import assert from "node:assert/strict";
import events from "node:events";
import jwt from "jsonwebtoken";
import { TestControlConnector, TestHostConnector, TestControl, get, exec, url, slowTest } from "./index.js";
import {
TestControlConnector, TestHostConnector, TestControl, get, exec, url, controlToken, slowTest,
} from "./index.js";
import { ConnectionClosed, ProtocolError, PolicyViolation, AuthenticationFailed } from "@clusterio/lib";
let token = jwt.sign({ aud: "user", user: "test" }, Buffer.from("TestSecretDoNotUse", "base64"));
let tokenHost = jwt.sign({ aud: "host", host: 0 }, Buffer.from("TestSecretDoNotUse", "base64"));
describe("Integration of lib/link", function() {
@@ -14,7 +15,7 @@ describe("Integration of lib/link", function() {
let hostConnector;
beforeEach(async function() {
controlConnector = new TestControlConnector(url, 0.2);
controlConnector.token = token;
controlConnector.token = controlToken;
control = new TestControl(controlConnector, false);
hostConnector = new TestHostConnector(url, 0.2);
hostConnector.token = tokenHost;
@@ -100,7 +101,7 @@ describe("Integration of lib/link", function() {
});
it("should refuse connection with incorrect audience", async function() {
hostConnector.token = token;
hostConnector.token = controlToken;
await assert.rejects(
hostConnector.connect(),
new Error("Authentication failed: jwt audience invalid. expected: host or slave")
+27
View File
@@ -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"]);
});
});
});
+45
View File
@@ -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,