fix(sidecar): refuse a plugin that names another server (D155)

`[game].server_id` was a cross-check that WARNED and kept the plugin's id.
The phase 18 walk showed what that costs: a second server's plugin,
parked in this listener's backlog by a plugin bug (Rust-Plugins, D154),
was accepted the moment the first server's plugin reloaded, and the
website showed server "alpha" with beta's hostname and wipe.

Now, with `server_id` set, the connection is closed on the first frame
that names another server, BEFORE that frame reaches the store or the
feed, and both ids are logged at ERROR. The command channel is installed
only once a frame has named this server, so no website command (a grant,
a world write) can reach a plugin about to be refused, and /health reports
the plugin connected only from then. Blank `server_id`: nothing checked,
as before.

The egg's launcher now hands the sidecar the plugin config's ServerId once
that file exists. The plugin reads RUSTLINK_SERVER_ID only at its first
config write (D150); without this, a variable edited after the first boot
would be refused instead of changing nothing, as INSTALL.md promises.

Walked: a plugin aimed at another server's sidecar is refused with an
ERROR naming both ids, and the site keeps the right server's identity.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E14m6SuuY6i1vASFeGDBeY

egg/with-sidecar.sh

@@ -31,6 +31,23 @@ for v in RUSTLINK_SERVER_ID RUSTLINK_WEB_TOKEN RUSTLINK_RETAIN_DAYS RUSTLINK_WEB
   if [ -z "$val" ]; then unset "$v"; fi
 done
 
+# The server id follows the PLUGIN's config once it exists. The plugin reads RUSTLINK_SERVER_ID only
+# when it writes its first config; after that the file is canonical and the website locks it (D150).
+# The sidecar refuses a plugin that names another server (D155), so a variable edited after the
+# first boot would otherwise take the bridge down instead of changing nothing. Stock framework paths,
+# the same ones the installer writes. `sed`, because the game image has no jq.
+for f in /home/container/oxide/config/RunicGateway.json /home/container/carbon/configs/RunicGateway.json; do
+  [ -f "$f" ] || continue
+  ID="$(sed -n 's/^[[:space:]]*"ServerId":[[:space:]]*"\([^"]*\)".*$/\1/p' "$f" | head -n 1)"
+  if [ -n "$ID" ]; then
+    if [ -n "${RUSTLINK_SERVER_ID-}" ] && [ "$RUSTLINK_SERVER_ID" != "$ID" ]; then
+      echo "[rust-link] RUSTLINK_SERVER_ID is '$RUSTLINK_SERVER_ID' but the plugin's config names '$ID'; using '$ID', the id the website knows. The variable is read once, at the first boot."
+    fi
+    export RUSTLINK_SERVER_ID="$ID"
+  fi
+  break
+done
+
 # The website-facing bind. Pterodactyl tells a container nothing about its extra allocations, so
 # the port is typed into the egg, and it must be one of this server's allocations — a port that is
 # not fails as a bind the website never reaches, which the lines below make visible (§34.1).

sidecar/src/app.rs

@@ -87,7 +87,8 @@ where
 
     // Game link: events in, commands out.
     let (event_tx, mut event_rx) = mpsc::unbounded_channel::<game::GameEvent>();
-    let (handle, _bound) = game::serve(&cfg.game.bind, event_tx).await?;
+    let (handle, _bound) =
+        game::serve(&cfg.game.bind, cfg.game.server_id.clone(), event_tx).await?;
 
     // Live feed: every game event fans out to all connected website WebSocket clients.
     let (bcast_tx, _) = broadcast::channel::<String>(1024);
@@ -145,7 +146,6 @@ where
     // waiting caller and stop. Everything else is filed by its `type`, and by its `type` alone:
     // that is what keeps this process a dumb forwarder while the catalogue grows (PROTOCOL.md
     // §8.1). Ten new event kinds are no change here.
-    let configured_server_id = cfg.game.server_id.clone();
     let feed_tx = bcast_tx.clone();
     let route_rpc = rpc.clone();
     let event_store = store.clone();
@@ -194,7 +194,6 @@ where
                     // `type` exists to prevent, and it is invisible until somebody wonders why the
                     // presence board has four thousand rows.
                     if ev.kind == store::SERVER_BOARD {
-                        check_server_id(&configured_server_id, &ev.value);
                         info!(kind = %ev.kind, "{}", line);
                     } else {
                         tracing::debug!(kind = %ev.kind, "board {}", line);
@@ -260,24 +259,6 @@ where
     Ok(())
 }
 
-/// The plugin's `serverId` is the authority; `[game].server_id` is a cross-check. A disagreement is
-/// almost always two game servers pointed at one sidecar by a copied config, which is silent in
-/// every other design and produces one server's history under another's name.
-fn check_server_id(configured: &str, hello: &serde_json::Value) {
-    if configured.is_empty() {
-        return;
-    }
-    let announced = hello.get("serverId").and_then(|v| v.as_str()).unwrap_or("");
-    if !announced.is_empty() && announced != configured {
-        warn!(
-            configured,
-            announced,
-            "the connected plugin announces a different serverId than this sidecar is configured \
-             for; keeping the plugin's. Two servers sharing one sidecar is the usual cause."
-        );
-    }
-}
-
 /// A frame's own timestamp, falling back to ours. The plugin stamps `t` at the moment the world was
 /// read, which is earlier than the moment we saw the line and is the one worth keeping.
 fn frame_t(value: &serde_json::Value) -> i64 {

sidecar/src/config.rs

@@ -17,9 +17,11 @@
 //! R8 makes the platform multi-server: one sidecar per game server, and every row the module
 //! stores carries the server it came from. The plugin declares its own `serverId` in `server.hello`
 //! and that is the authority. This setting is a **cross-check**, not a second source of truth: when
-//! both are set and they disagree, the sidecar logs the disagreement loudly and keeps the plugin's.
+//! both are set and they disagree, the sidecar **refuses the plugin** — it closes the connection
-//! Two servers pointed at one sidecar by a copy-pasted config is the mistake this catches, and it
+//! before the frame is filed, and logs both ids at ERROR (D155). Two servers dialling one sidecar is
-//! is silent in every other design.
+//! the mistake this catches, and it is silent in every other design. It was a warning that kept the
+//! plugin's id until the phase 18 walk showed what that costs: one server's history filed under
+//! another's name, visible on the website. Left blank, nothing is checked.
 
 use std::env;
 use std::fs;

sidecar/src/game.rs

@@ -23,7 +23,7 @@ use serde_json::Value;
 use tokio::io::{AsyncBufRead, AsyncBufReadExt, AsyncWriteExt, BufReader};
 use tokio::net::TcpListener;
 use tokio::sync::{mpsc, Mutex};
-use tracing::{info, warn};
+use tracing::{error, info, warn};
 
 /// The longest line the sidecar will accept from the plugin, in bytes.
 ///
@@ -181,8 +181,12 @@ impl GameHandle {
 /// The **bound** address is returned alongside the handle rather than assumed to be the one asked
 /// for: `127.0.0.1:0` is a legitimate thing to configure (and what the tests use), and a log line
 /// echoing the request rather than the result is the kind that is wrong exactly when it matters.
+///
+/// `server_id` is `[game].server_id`. When it is set, a plugin that names another server is
+/// refused — see [`foreign_server_id`].
 pub async fn serve(
     addr: &str,
+    server_id: String,
     event_tx: mpsc::UnboundedSender<GameEvent>,
 ) -> std::io::Result<(GameHandle, std::net::SocketAddr)> {
     let listener = TcpListener::bind(addr).await?;
@@ -197,7 +201,9 @@ pub async fn serve(
             match listener.accept().await {
                 Ok((stream, peer)) => {
                     info!(%peer, "plugin connected");
-                    if let Err(e) = handle_connection(stream, &event_tx, &accept_handle).await {
+                    if let Err(e) =
+                        handle_connection(stream, &server_id, &event_tx, &accept_handle).await
+                    {
                         warn!(error = %e, "plugin connection ended");
                     } else {
                         info!("plugin disconnected");
@@ -224,17 +230,44 @@ pub async fn serve(
     Ok((handle, bound))
 }
 
+/// The server a frame names, when it is not the one this sidecar is configured for.
+///
+/// `None` means the frame may pass: no `server_id` is configured, or the frame names none, or it
+/// names this one. The plugin stamps `serverId` on every frame, so the first line of a connection
+/// is enough to tell — and the check runs before that line reaches the store or the feed, because
+/// by the time it is filed, one server's history is already under another's name (D155). That is
+/// what the phase 18 walk saw when this was a warning: a second server's plugin, parked in this
+/// listener's backlog, was accepted the moment the first one reloaded, and the website showed the
+/// first server with the second's hostname and wipe.
+fn foreign_server_id<'a>(configured: &str, frame: &'a Value) -> Option<&'a str> {
+    if configured.is_empty() {
+        return None;
+    }
+    match frame.get("serverId").and_then(|v| v.as_str()) {
+        Some(announced) if !announced.is_empty() && announced != configured => Some(announced),
+        _ => None,
+    }
+}
+
 async fn handle_connection(
     stream: tokio::net::TcpStream,
+    server_id: &str,
     event_tx: &mpsc::UnboundedSender<GameEvent>,
     handle: &GameHandle,
 ) -> std::io::Result<()> {
     stream.set_nodelay(true).ok();
     let (read_half, mut write_half) = stream.into_split();
 
-    // Install the outbound command channel for this connection.
+    // The outbound command channel for this connection. With a `server_id` configured it is
+    // installed only once a frame has named this server: until then the peer could be a plugin
+    // about to be refused, and a command sent in that window — a permission grant, a world write —
+    // would land on the wrong server. `/health` reads the same handle, so it reports the plugin
+    // connected only from that moment too.
     let (cmd_tx, mut cmd_rx) = mpsc::unbounded_channel::<String>();
-    handle.set(Some(cmd_tx)).await;
+    let mut pending_tx = Some(cmd_tx);
+    if server_id.is_empty() {
+        handle.set(pending_tx.take()).await;
+    }
 
     let mut reader = BufReader::new(read_half);
     let mut lines = LineReader::default();
@@ -258,6 +291,24 @@ async fn handle_connection(
                         if !trimmed.is_empty() {
                             match serde_json::from_str::<Value>(trimmed) {
                                 Ok(value) => {
+                                    if let Some(announced) = foreign_server_id(server_id, &value) {
+                                        error!(
+                                            configured = server_id,
+                                            announced,
+                                            "refusing a plugin that names another server: this \
+                                             sidecar is '{server_id}', the plugin says \
+                                             '{announced}'. Two servers are dialling one game \
+                                             port — check each plugin config's Port against its \
+                                             sidecar's [game].bind"
+                                        );
+                                        return Ok(());
+                                    }
+                                    if pending_tx.is_some()
+                                        && value.get("serverId").and_then(|v| v.as_str())
+                                            == Some(server_id)
+                                    {
+                                        handle.set(pending_tx.take()).await;
+                                    }
                                     let kind = value
                                         .get("kind")
                                         .and_then(|k| k.as_str())
@@ -396,7 +447,7 @@ mod tests {
         use tokio::io::AsyncReadExt;
 
         let (tx, mut rx) = mpsc::unbounded_channel();
-        let (handle, addr) = serve("127.0.0.1:0", tx).await.unwrap();
+        let (handle, addr) = serve("127.0.0.1:0", String::new(), tx).await.unwrap();
 
         let mut client = tokio::net::TcpStream::connect(addr).await.unwrap();
         client
@@ -415,4 +466,67 @@ mod tests {
         let n = client.read(&mut buf).await.unwrap();
         assert_eq!(&buf[..n], b"{\"cmd\":\"ping\"}\n");
     }
+
+    #[test]
+    fn foreign_server_id_names_only_a_disagreement() {
+        let beta = serde_json::json!({ "kind": "server.hello", "serverId": "beta" });
+        let alpha = serde_json::json!({ "kind": "server.hello", "serverId": "alpha" });
+        let anonymous = serde_json::json!({ "kind": "pong" });
+        let blank = serde_json::json!({ "kind": "pong", "serverId": "" });
+
+        assert_eq!(foreign_server_id("alpha", &beta), Some("beta"));
+        assert_eq!(foreign_server_id("alpha", &alpha), None);
+        assert_eq!(foreign_server_id("alpha", &anonymous), None);
+        assert_eq!(foreign_server_id("alpha", &blank), None);
+        // Nothing configured: nothing to check against.
+        assert_eq!(foreign_server_id("", &beta), None);
+    }
+
+    /// D155, over a real socket: the plugin that names another server is closed on its first
+    /// frame, the frame is never forwarded, and no command could have reached it meanwhile.
+    #[tokio::test]
+    async fn a_plugin_naming_another_server_is_refused_before_anything_is_filed() {
+        use tokio::io::AsyncReadExt;
+
+        let (tx, mut rx) = mpsc::unbounded_channel();
+        let (handle, addr) = serve("127.0.0.1:0", "alpha".to_string(), tx).await.unwrap();
+
+        let mut client = tokio::net::TcpStream::connect(addr).await.unwrap();
+        // Accepted, but not yet vetted: no command may go to it.
+        tokio::time::sleep(std::time::Duration::from_millis(50)).await;
+        assert!(!handle.is_connected().await);
+
+        client
+            .write_all(b"{\"kind\":\"server.hello\",\"type\":\"board\",\"serverId\":\"beta\"}\n")
+            .await
+            .unwrap();
+
+        let mut buf = [0u8; 16];
+        let n = tokio::time::timeout(std::time::Duration::from_secs(2), client.read(&mut buf))
+            .await
+            .expect("the sidecar closes a refused plugin")
+            .unwrap_or(0);
+        assert_eq!(n, 0);
+
+        // The only thing forwarded is the listener's own link.down, never beta's hello.
+        let ev = rx.recv().await.unwrap();
+        assert_eq!(ev.kind, "link.down");
+        assert!(!handle.is_connected().await);
+    }
+
+    #[tokio::test]
+    async fn the_command_channel_opens_on_the_first_frame_naming_this_server() {
+        let (tx, mut rx) = mpsc::unbounded_channel();
+        let (handle, addr) = serve("127.0.0.1:0", "alpha".to_string(), tx).await.unwrap();
+
+        let mut client = tokio::net::TcpStream::connect(addr).await.unwrap();
+        client
+            .write_all(b"{\"kind\":\"server.hello\",\"serverId\":\"alpha\"}\n")
+            .await
+            .unwrap();
+
+        let ev = rx.recv().await.unwrap();
+        assert_eq!(ev.value["serverId"], "alpha");
+        assert!(handle.is_connected().await);
+    }
 }