Merge pull request 'feat(rust): install the helpers a plugin release ships beside the bridge (D182)' (#35) from feat/rust-plugin-helpers into main

Reviewed-on: #35
Reviewed-by: Colby Whitlock <whitlocktech@gmail.com>

src/rustgame/doctor.rs

@@ -4,6 +4,8 @@
 //! says where to look. Per instance (docs/modules/rust/PLAN.md §34.2.3):
 //!
 //! - the framework, and whether the plugin file is still the one deployed;
+//! - each helper deployed beside it (D182), as a warning when it is gone or edited — the bridge
+//!   runs without one, and the row says what that costs;
 //! - that the plugin's config names this instance;
 //! - `requires_plugins` present, as a warning (D153);
 //! - the service registered and running;
@@ -151,6 +153,31 @@ fn instance_rows(
         )),
     }
 
+    // ── The helpers: ours too, and optional to the bridge (D182) ─────────────
+    // A warning rather than a failure: the bridge runs without a helper and falls back. What is
+    // lost is said, so an operator who removed one on purpose knows what they chose.
+    for (name, helper) in &instance.helpers {
+        match std::fs::read(&helper.path) {
+            Ok(bytes) if crate::util::sha256_bytes(&bytes) == helper.sha256 => {
+                rows.push(Row::ok(&label("helper"), helper.path.clone()))
+            }
+            Ok(_) => rows.push(
+                Row::warn(
+                    &label("helper"),
+                    format!("{} is not the file that was deployed", helper.path),
+                )
+                .note(
+                    "edited or replaced by hand; `update --game rust` puts the released one back",
+                ),
+            ),
+            Err(_) => rows.push(
+                Row::warn(&label("helper"), format!("{} is missing", helper.path))
+                    .note(helper_absence(name))
+                    .note("`update --game rust` puts it back"),
+            ),
+        }
+    }
+
     // ── The plugin's config: the website's ───────────────────────────────────
     match plugin::read_config(Path::new(&instance.plugin_config)) {
         Ok(Some(view)) if view.server_id.as_deref() == Some(id) => rows.push(Row::ok(
@@ -279,6 +306,18 @@ fn instance_rows(
 }
 
 /// Whether a newer Rust bundle than the installed one is published.
+/// What a missing helper costs, in the operator's words. Unknown helpers get a generic line.
+fn helper_absence(name: &str) -> String {
+    match name {
+        "RunicGatewayZones.cs" => {
+            "the bridge still runs and scores its zones by position, but ZoneManager's \
+             own flags miss anybody already standing in a zone when it is created or restored"
+                .to_string()
+        }
+        _ => format!("the bridge still runs without {name}; what it helped with falls back"),
+    }
+}
+
 fn bundle_row(record: &RustRecord) -> Row {
     match bundle::fetch_rust(None) {
         Ok((current, _)) if current.bundle == record.bundle.tag => {

src/rustgame/install.rs

@@ -19,7 +19,7 @@ use std::path::{Path, PathBuf};
 use anyhow::{bail, Context, Result};
 
 use super::plugin::{self, ConfigView};
-use super::record::{ComponentRecord, Instance, RustRecord, SCHEMA};
+use super::record::{ComponentRecord, DeployedFile, Instance, RustRecord, SCHEMA};
 use super::server::{self, RustServer};
 use super::sidecar;
 use crate::bundle::{self, RustBundle};
@@ -42,6 +42,10 @@ struct Planned {
     running: bool,
     plugin_config: Option<ConfigView>,
     plugin_action: BinaryAction,
+    /// Each helper in the release and what happens to it, in the release's (name) order.
+    helper_actions: Vec<BinaryAction>,
+    /// Helpers this instance's record holds that the release no longer ships: removed.
+    retired_helpers: Vec<String>,
     game_port: u16,
     web_port: u16,
     config_path: PathBuf,
@@ -179,7 +183,7 @@ pub fn deploy(cli: &Cli, mode: Mode) -> Result<()> {
         )?);
     }
     let binary_action = crate::sidecar::decide(&asset, &layout.rust_sidecar_bin)?;
-    print_plan(&layout, &planned, binary_action, &bundle);
+    print_plan(&layout, &planned, binary_action, &bundle, &released);
 
     if cli.verify {
         println!(
@@ -326,7 +330,14 @@ pub fn deploy(cli: &Cli, mode: Mode) -> Result<()> {
                 Some(b) if b.tag == bundle.bundle => {
                     // The same bundle can still have written something: a plugin edited by hand is
                     // put back, which is what `doctor` tells an operator to run `update` for.
-                    let restored = planned.iter().filter(|p| p.plugin_action.writes()).count();
+                    let restored = planned
+                        .iter()
+                        .filter(|p| {
+                            p.plugin_action.writes()
+                                || p.helper_actions.iter().any(|a| a.writes())
+                                || !p.retired_helpers.is_empty()
+                        })
+                        .count();
                     if restored == 0 && !binary_action.writes() {
                         println!("\nAlready on bundle {} — nothing moved.", bundle.bundle)
                     } else {
@@ -431,6 +442,30 @@ fn plan_instance(
         }
         Ok(_) => BinaryAction::Replace,
     };
+    // Helpers are decided the same way as the plugin: absent, identical or replaced. One the
+    // operator deleted is put back — the installer owns it, and `doctor` says what its absence costs.
+    let helper_actions = released
+        .helpers
+        .iter()
+        .map(
+            |helper| match std::fs::read(server.plugins_dir().join(&helper.name)) {
+                Err(_) => BinaryAction::Install,
+                Ok(bytes) if crate::util::sha256_bytes(&bytes) == helper.sha256 => {
+                    BinaryAction::Unchanged
+                }
+                Ok(_) => BinaryAction::Replace,
+            },
+        )
+        .collect();
+    let retired_helpers = recorded
+        .map(|i| {
+            i.helpers
+                .keys()
+                .filter(|name| !released.helpers.iter().any(|h| &h.name == *name))
+                .cloned()
+                .collect()
+        })
+        .unwrap_or_default();
     let config_path = layout.rust_config(id);
     Ok(Planned {
         id: id.to_string(),
@@ -441,6 +476,8 @@ fn plan_instance(
         server,
         plugin_config,
         plugin_action,
+        helper_actions,
+        retired_helpers,
         game_port,
         web_port,
     })
@@ -451,6 +488,7 @@ fn print_plan(
     planned: &[Planned],
     binary: BinaryAction,
     bundle: &RustBundle,
+    released: &plugin::Released,
 ) {
     ui::row(
         "binary",
@@ -479,6 +517,25 @@ fn print_plan(
                 p.plugin_action.label()
             ),
         );
+        for (helper, action) in released.helpers.iter().zip(&p.helper_actions) {
+            ui::row(
+                "helper",
+                &format!(
+                    "{}   {}",
+                    p.server.plugins_dir().join(&helper.name).display(),
+                    action.label()
+                ),
+            );
+        }
+        for name in &p.retired_helpers {
+            ui::row(
+                "helper",
+                &format!(
+                    "{}   removed (no longer released)",
+                    p.server.plugins_dir().join(name).display()
+                ),
+            );
+        }
         ui::row(
             "plugin config",
             &match &p.plugin_config {
@@ -662,6 +719,45 @@ fn deploy_instance(
         }
     };
 
+    // The helpers before the plugin (D182): each loads the moment it lands, and the bridge reads a
+    // helper's state at hello — a helper already there is one the bridge's first hello reports.
+    // Written in place for the plugin's reason below.
+    let mut helpers = std::collections::BTreeMap::new();
+    for (helper, action) in released.helpers.iter().zip(&plan.helper_actions) {
+        let path = plan.server.plugins_dir().join(&helper.name);
+        if action.writes() {
+            std::fs::create_dir_all(plan.server.plugins_dir()).with_context(|| {
+                format!("cannot create {}", plan.server.plugins_dir().display())
+            })?;
+            std::fs::write(&path, &helper.source)
+                .with_context(|| format!("cannot write {}", path.display()))?;
+            ui::ok(&format!(
+                "helper {}   {}",
+                if *action == BinaryAction::Replace {
+                    "replaced"
+                } else {
+                    "installed"
+                },
+                path.display()
+            ));
+        }
+        helpers.insert(
+            helper.name.clone(),
+            DeployedFile {
+                path: path.display().to_string(),
+                sha256: helper.sha256.clone(),
+            },
+        );
+    }
+    for name in &plan.retired_helpers {
+        let path = plan.server.plugins_dir().join(name);
+        match std::fs::remove_file(&path) {
+            Ok(()) => ui::ok(&format!("helper removed   {}", path.display())),
+            Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
+            Err(e) => ui::warn(&format!("could not remove {}: {e}", path.display())),
+        }
+    }
+
     // The plugin last: it loads the moment it lands, and its sidecar is now there to dial.
     let plugin_path = plan.server.plugin_path();
     if plan.plugin_action.writes() {
@@ -694,6 +790,7 @@ fn deploy_instance(
             framework: plan.server.framework.as_str().to_string(),
             plugin_path: plugin_path.display().to_string(),
             plugin_sha256: released.sha256.clone(),
+            helpers,
             plugin_config: plugin_config_path.display().to_string(),
             plugin_config_written: wrote_plugin_config || plugin_config_written_before,
             game_port: plan.game_port,

src/rustgame/plugin.rs

@@ -1,9 +1,12 @@
 //! The plugin: its released tarball, and its config in the server root.
 //!
-//! Two files, two owners (docs/modules/rust/PLAN.md §34.2.3):
+//! Two kinds of file, two owners (docs/modules/rust/PLAN.md §34.2.3):
 //!
 //! - **`RunicGateway.cs` is the installer's.** It comes from the bundle, is replaced when the bundle
-//!   moves, and is removed by `uninstall`.
+//!   moves, and is removed by `uninstall`. So are the **helpers** released beside it
+//!   (docs/modules/rust/PLAN_FIXES.md D168, D182 — today `RunicGatewayZones.cs`): every other `.cs`
+//!   the release's manifest lists in `files`, each checked against its own sha256. A helper is
+//!   optional to the bridge at runtime and installed by default.
 //! - **`RunicGateway.json` is the website's.** The plugin writes it, the site edits it through the
 //!   plugin, and it locks `ServerId`. The installer writes it exactly once — when it does not exist
 //!   yet, holding just `ServerId` and `Port` — and never rewrites it. An existing one whose
@@ -37,6 +40,28 @@ pub struct Released {
     pub manifest: Manifest,
     pub source: Vec<u8>,
     pub sha256: String,
+    /// The helpers the manifest lists, in name order. Empty for a release older than them.
+    pub helpers: Vec<Helper>,
+}
+
+/// One helper plugin shipped beside the bridge (D168, D182).
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct Helper {
+    /// The file name, as the manifest and the plugins directory both spell it.
+    pub name: String,
+    pub source: Vec<u8>,
+    pub sha256: String,
+}
+
+/// A helper's name is written into a plugins directory, so it must be a plain `<Name>.cs` and
+/// nothing a tarball could use to reach outside that directory.
+pub fn is_helper_name(name: &str) -> bool {
+    let Some(stem) = name.strip_suffix(".cs") else {
+        return false;
+    };
+    !stem.is_empty()
+        && name != super::server::PLUGIN_FILE
+        && stem.chars().all(|c| c.is_ascii_alphanumeric() || c == '_')
 }
 
 /// Reads the plugin and its manifest out of a downloaded tarball, and checks that the two agree.
@@ -46,22 +71,27 @@ pub fn read_tarball(path: &Path) -> Result<Released> {
     let mut archive = tar::Archive::new(flate2::read::GzDecoder::new(file));
     let mut manifest: Option<Vec<u8>> = None;
     let mut source: Option<Vec<u8>> = None;
+    let mut others: BTreeMap<String, Vec<u8>> = BTreeMap::new();
     for entry in archive
         .entries()
         .context("the plugin tarball is not a tar.gz")?
     {
         let mut entry = entry.context("the plugin tarball is truncated")?;
         let name = entry.path()?.to_string_lossy().replace('\\', "/");
-        let slot = if name == format!("{PREFIX}/manifest.json") {
-            &mut manifest
-        } else if name == format!("{PREFIX}/{}", super::server::PLUGIN_FILE) {
-            &mut source
-        } else {
-            continue;
-        };
         let mut bytes = Vec::new();
-        entry.read_to_end(&mut bytes)?;
-        *slot = Some(bytes);
+        if name == format!("{PREFIX}/manifest.json") {
+            entry.read_to_end(&mut bytes)?;
+            manifest = Some(bytes);
+        } else if name == format!("{PREFIX}/{}", super::server::PLUGIN_FILE) {
+            entry.read_to_end(&mut bytes)?;
+            source = Some(bytes);
+        } else if let Some(file) = name.strip_prefix(&format!("{PREFIX}/")) {
+            // Kept only if it is a plain helper name; which of them count is the manifest's call.
+            if is_helper_name(file) {
+                entry.read_to_end(&mut bytes)?;
+                others.insert(file.to_string(), bytes);
+            }
+        }
     }
 
     let manifest = manifest
@@ -84,13 +114,51 @@ pub fn read_tarball(path: &Path) -> Result<Released> {
             );
         }
     }
+    let helpers = read_helpers(&manifest, others)?;
     Ok(Released {
         manifest,
         source,
         sha256,
+        helpers,
     })
 }
 
+/// Every file the manifest lists besides the bridge, each present and matching its sha256. A name
+/// the manifest lists that is not a plain helper name refuses the release: the installer would
+/// otherwise be asked to write it somewhere, and it will not guess where.
+fn read_helpers(manifest: &Manifest, mut found: BTreeMap<String, Vec<u8>>) -> Result<Vec<Helper>> {
+    let mut helpers = Vec::new();
+    for (name, declared) in &manifest.files {
+        if name == super::server::PLUGIN_FILE {
+            continue;
+        }
+        if !is_helper_name(name) {
+            bail!(
+                "the plugin's manifest lists {name:?}, which is not a plugin file name this \
+                 installer will write into a plugins directory — refusing the release."
+            );
+        }
+        let source = found.remove(name).ok_or_else(|| {
+            anyhow::anyhow!(
+                "the plugin's manifest lists {name} but the tarball has no {PREFIX}/{name}"
+            )
+        })?;
+        let sha256 = sha256_bytes(&source);
+        if !declared.eq_ignore_ascii_case(&sha256) {
+            bail!(
+                "{name} in the plugin tarball does not match its own manifest (sha256 {sha256}, \
+                 manifest says {declared}) — refusing the release."
+            );
+        }
+        helpers.push(Helper {
+            name: name.clone(),
+            source,
+            sha256,
+        });
+    }
+    Ok(helpers)
+}
+
 /// What the plugin's config says, as far as the installer cares.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct ConfigView {
@@ -228,6 +296,133 @@ mod tests {
         assert_eq!(view.port, Some(7799));
     }
 
+    /// A plugin tarball shaped like Rust-Plugins' release: `runicgateway-rust-plugin/<name>` entries.
+    fn tarball(dir: &Path, entries: &[(&str, &[u8])]) -> std::path::PathBuf {
+        let path = dir.join("plugin.tar.gz");
+        let file = std::fs::File::create(&path).unwrap();
+        let mut builder = tar::Builder::new(flate2::write::GzEncoder::new(
+            file,
+            flate2::Compression::default(),
+        ));
+        for (name, bytes) in entries {
+            let mut header = tar::Header::new_gnu();
+            header.set_size(bytes.len() as u64);
+            header.set_mode(0o644);
+            header.set_cksum();
+            builder
+                .append_data(&mut header, format!("{PREFIX}/{name}"), *bytes)
+                .unwrap();
+        }
+        builder.into_inner().unwrap().finish().unwrap();
+        path
+    }
+
+    fn manifest(files: &[(&str, &[u8])]) -> Vec<u8> {
+        let files: serde_json::Map<String, serde_json::Value> = files
+            .iter()
+            .map(|(name, bytes)| ((*name).to_string(), sha256_bytes(bytes).into()))
+            .collect();
+        serde_json::to_vec(
+            &serde_json::json!({ "version": "0.2.0", "protocol": 13, "files": files }),
+        )
+        .unwrap()
+    }
+
+    const BRIDGE: &[u8] = b"// the bridge";
+    const ZONES: &[u8] = b"// the zone helper";
+
+    #[test]
+    fn a_release_with_a_helper_carries_it_checked() {
+        let dir = TempDir::new("rg-rust-helper").unwrap();
+        let m = manifest(&[("RunicGateway.cs", BRIDGE), ("RunicGatewayZones.cs", ZONES)]);
+        let path = tarball(
+            dir.path(),
+            &[
+                ("manifest.json", &m),
+                ("RunicGateway.cs", BRIDGE),
+                ("RunicGatewayZones.cs", ZONES),
+            ],
+        );
+        let released = read_tarball(&path).unwrap();
+        assert_eq!(released.source, BRIDGE);
+        assert_eq!(released.helpers.len(), 1);
+        assert_eq!(released.helpers[0].name, "RunicGatewayZones.cs");
+        assert_eq!(released.helpers[0].source, ZONES);
+        assert_eq!(released.helpers[0].sha256, sha256_bytes(ZONES));
+    }
+
+    #[test]
+    fn a_release_older_than_helpers_has_none_even_with_a_stray_file() {
+        // Only what the manifest lists is a helper: a `.cs` it does not name is not installed.
+        let dir = TempDir::new("rg-rust-helper-old").unwrap();
+        let m = manifest(&[("RunicGateway.cs", BRIDGE)]);
+        let path = tarball(
+            dir.path(),
+            &[
+                ("manifest.json", &m),
+                ("RunicGateway.cs", BRIDGE),
+                ("RunicGatewayZones.cs", ZONES),
+            ],
+        );
+        assert!(read_tarball(&path).unwrap().helpers.is_empty());
+    }
+
+    #[test]
+    fn a_helper_the_manifest_lists_must_be_there_and_match() {
+        let dir = TempDir::new("rg-rust-helper-bad").unwrap();
+        let m = manifest(&[("RunicGateway.cs", BRIDGE), ("RunicGatewayZones.cs", ZONES)]);
+
+        let missing = tarball(
+            dir.path(),
+            &[("manifest.json", &m), ("RunicGateway.cs", BRIDGE)],
+        );
+        let err = read_tarball(&missing).unwrap_err().to_string();
+        assert!(err.contains("RunicGatewayZones.cs"), "{err}");
+
+        let tampered = tarball(
+            dir.path(),
+            &[
+                ("manifest.json", &m),
+                ("RunicGateway.cs", BRIDGE),
+                ("RunicGatewayZones.cs", b"// something else"),
+            ],
+        );
+        let err = read_tarball(&tampered).unwrap_err().to_string();
+        assert!(err.contains("does not match its own manifest"), "{err}");
+    }
+
+    #[test]
+    fn a_manifest_naming_a_path_is_refused() {
+        let dir = TempDir::new("rg-rust-helper-path").unwrap();
+        for name in ["../Evil.cs", "sub/Evil.cs", "Evil.dll", ".cs"] {
+            let m = manifest(&[("RunicGateway.cs", BRIDGE), (name, ZONES)]);
+            let path = tarball(
+                dir.path(),
+                &[("manifest.json", &m), ("RunicGateway.cs", BRIDGE)],
+            );
+            let err = read_tarball(&path).unwrap_err().to_string();
+            assert!(err.contains("refusing the release"), "{name}: {err}");
+        }
+    }
+
+    #[test]
+    fn helper_names_are_plain_plugin_files() {
+        assert!(is_helper_name("RunicGatewayZones.cs"));
+        assert!(is_helper_name("Helper_2.cs"));
+        for bad in [
+            "RunicGateway.cs",
+            "../X.cs",
+            "a/X.cs",
+            "a\\X.cs",
+            "X.cs.bak",
+            ".cs",
+            "X .cs",
+            "X-y.cs",
+        ] {
+            assert!(!is_helper_name(bad), "{bad}");
+        }
+    }
+
     #[test]
     fn a_missing_required_plugin_is_named() {
         let dir = TempDir::new("rg-rust-req").unwrap();

src/rustgame/record.rs

@@ -60,6 +60,10 @@ pub struct Instance {
     pub plugin_path: String,
     /// What was deployed, so `doctor` can tell an edited plugin file from the release's.
     pub plugin_sha256: String,
+    /// The helpers deployed beside the plugin (D168, D182), by file name. Installer-owned like the
+    /// plugin. Absent from a record written before helpers existed, which reads back as none.
+    #[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
+    pub helpers: BTreeMap<String, DeployedFile>,
     /// The plugin's config — the website's file, recorded so `doctor` knows where to look.
     pub plugin_config: String,
     /// The installer wrote that config (it did not exist). Informational; it is kept either way.
@@ -75,6 +79,13 @@ pub struct Instance {
     pub service: Option<ServiceRecord>,
 }
 
+/// One installer-owned file in a server root, as deployed.
+#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
+pub struct DeployedFile {
+    pub path: String,
+    pub sha256: String,
+}
+
 impl RustRecord {
     /// Everything but the timestamp, so a second run with nothing to do writes nothing.
     pub fn same_deployment_as(&self, other: &Self) -> bool {
@@ -142,6 +153,7 @@ mod tests {
             framework: "oxide".into(),
             plugin_path: format!("{root}/oxide/plugins/RunicGateway.cs"),
             plugin_sha256: "ab".repeat(32),
+            helpers: BTreeMap::new(),
             plugin_config: format!("{root}/oxide/config/RunicGateway.json"),
             plugin_config_written: true,
             game_port: game,
@@ -199,6 +211,25 @@ mod tests {
         assert!(!text.contains("auth_token") && !text.contains("token\""));
     }
 
+    #[test]
+    fn a_record_written_before_helpers_loads_with_none() {
+        // Every host installed before D182 has one of these: no `helpers` key at all.
+        let dir = TempDir::new("rg-rust-record-old").unwrap();
+        let path = dir.path().join("install.json");
+        let mut value = serde_json::to_value(sample()).unwrap();
+        for instance in value["instances"].as_object_mut().unwrap().values_mut() {
+            assert!(instance
+                .as_object_mut()
+                .unwrap()
+                .remove("helpers")
+                .is_none());
+        }
+        std::fs::write(&path, serde_json::to_vec(&value).unwrap()).unwrap();
+        let record = RustRecord::load(&path).unwrap().unwrap();
+        assert!(record.instances.values().all(|i| i.helpers.is_empty()));
+        assert_eq!(record, sample());
+    }
+
     #[test]
     fn ports_held_by_other_instances_exclude_the_one_being_installed() {
         let record = sample();

src/rustgame/sidecar.rs

@@ -185,7 +185,7 @@ pub fn handoff(
         protocol = doc.protocol,
         when = if running {
             format!(
-                "The plugin loads now; `doctor --game rust --server-id {server_id}` confirms it                  connected."
+                "The plugin loads now; `doctor --game rust --server-id {server_id}` confirms it connected."
             )
         } else {
             "The plugin connects when the server next starts.".to_string()
@@ -265,6 +265,16 @@ mod tests {
             running.contains("doctor --game rust --server-id alpha"),
             "{running}"
         );
+        // The whole first line, so a wrapped source line cannot leave its indent in the sentence
+        // again (installer#34: "confirms it                  connected.").
+        assert_eq!(
+            running.lines().find(|l| !l.is_empty()),
+            Some(
+                "Rust server \"alpha\" is set up. The plugin loads now; \
+                 `doctor --game rust --server-id alpha` confirms it connected."
+            ),
+            "{running}"
+        );
     }
 
     #[test]

src/rustgame/uninstall.rs

@@ -59,6 +59,9 @@ pub fn run(cli: &Cli) -> Result<i32> {
             println!("  · remove the service {}", svc.name);
         }
         println!("  · remove {}", i.plugin_path);
+        for helper in i.helpers.values() {
+            println!("  · remove {}", helper.path);
+        }
         println!(
             "  · keep   {}  (the website's; it names this server)",
             i.plugin_config
@@ -106,6 +109,11 @@ pub fn run(cli: &Cli) -> Result<i32> {
             done.extend(removal.done);
             problems.extend(removal.problems);
         }
+        // The helpers first: without the bridge they have nothing to do, and a helper left behind
+        // would keep patching ZoneManager for a bridge that is gone.
+        for helper in instance.helpers.values() {
+            remove_file(Path::new(&helper.path), &mut done, &mut problems);
+        }
         remove_file(Path::new(&instance.plugin_path), &mut done, &mut problems);
         if cli.purge {
             remove_file(Path::new(&instance.config_path), &mut done, &mut problems);