From 70eb073826dab1b40ee28022b665faf5a9fb8a04 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Sat, 26 Sep 2026 03:54:57 -0500 Subject: [PATCH] fix(service): start the Windows service after granting it its config (D157) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `sc start` ran inside registration, before `grant_service_access` let the new virtual account read `sidecar.toml` — which `protect_config` had locked to SYSTEM and Administrators. The first start died on "Access is denied (os error 5)" and the service sat STOPPED: the SCM's restart-on-failure policy never fires for a clean exit with an error code. A later manual start worked, because by then the grant existed. Found on the phase 18 walk (step 4, one Rust instance under the SCM), from the sidecar's own log. The code is shared, so the released ServUO installer has the same first-start failure; the org lead chose to fix both here (D157). Registration no longer starts the service; a new `start_registered` runs after the grant and refreshes the reported state. A no-op on Linux, where the service user exists before the config and registration starts the unit itself. Walked (Windows, elevated): install -> START_PENDING/running, /health on the first probe, clean stop and restart, doctor clean, uninstall clean. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01E14m6SuuY6i1vASFeGDBeY --- src/install.rs | 3 +- src/rustgame/install.rs | 3 +- src/service.rs | 71 ++++++++++++++++++++++++++++------------- 3 files changed, 53 insertions(+), 24 deletions(-) diff --git a/src/install.rs b/src/install.rs index 4b489e9..ae952d5 100644 --- a/src/install.rs +++ b/src/install.rs @@ -576,8 +576,9 @@ fn install_sidecar( layout.relocated, )?; - let outcome = service::register(&prepared, layout, action.writes())?; + let mut outcome = service::register(&prepared, layout, action.writes())?; service::grant_service_access(&config_path, &layout.data_dir, &outcome)?; + service::start_registered(&mut outcome, &layout.sidecar_bin, &config_path)?; let service_record = match &outcome { service::Outcome::Registered { kind, diff --git a/src/rustgame/install.rs b/src/rustgame/install.rs index 121ad0b..99c0046 100644 --- a/src/rustgame/install.rs +++ b/src/rustgame/install.rs @@ -624,7 +624,7 @@ fn deploy_instance( prepared.user.as_deref(), layout.relocated, )?; - let outcome = service::register_rust( + let mut outcome = service::register_rust( prepared, layout, &plan.id, @@ -632,6 +632,7 @@ fn deploy_instance( binary_changed, )?; service::grant_service_access(&plan.config_path, &data_dir, &outcome)?; + service::start_registered(&mut outcome, &layout.rust_sidecar_bin, &plan.config_path)?; let service_record = match &outcome { service::Outcome::Registered { kind, diff --git a/src/service.rs b/src/service.rs index 4b6071a..f21c2c1 100644 --- a/src/service.rs +++ b/src/service.rs @@ -399,22 +399,15 @@ pub fn windows_start_failure_for( #[cfg(windows)] fn register_windows(binary: &Path, config: &Path, restart: bool) -> Result { - register_windows_named( - WINDOWS_SERVICE, - DISPLAY_NAME, - MIN_SERVICE_SIDECAR, - binary, - config, - restart, - ) + register_windows_named(WINDOWS_SERVICE, DISPLAY_NAME, binary, config, restart) } -/// Registers (or reconfigures), sets the restart policy of, and starts one SCM service. +/// Registers (or reconfigures) one SCM service and sets its restart policy. It does not start it: +/// [`start_registered`] does, after [`grant_service_access`]. #[cfg(windows)] fn register_windows_named( name: &str, display: &str, - min_sidecar: &str, binary: &Path, config: &Path, restart: bool, @@ -473,17 +466,11 @@ fn register_windows_named( if restart && windows_service_state(name).contains("RUNNING") { stop_windows_service(name)?; } - // 1056 is ERROR_SERVICE_ALREADY_RUNNING, which is the desired end state, not a failure. - let start = run("sc.exe", &["start", name])?; - if !start.status.success() && start.status.code() != Some(1056) { - anyhow::bail!(windows_start_failure_for( - name, - min_sidecar, - start.status.code().unwrap_or(-1), - binary, - config - )); - } + // NOT started here: the virtual account `sc create` just made cannot read the config yet — + // `protect_config` locked it to SYSTEM and Administrators, and `grant_service_access` is what + // lets the account in. Started first, the sidecar died on `Access is denied` and the service + // sat STOPPED; the restart policy never fires for a clean exit with an error code. The phase 18 + // walk found it (D157). The caller grants, then calls `start_registered`. Ok(Outcome::Registered { kind: "windows-scm", @@ -659,7 +646,6 @@ pub fn register_rust( Manager::WindowsScm => register_windows_named( &rust_windows_service(server_id), &format!("Runic Gateway rust-link sidecar ({server_id})"), - MIN_RUST_SERVICE_SIDECAR, binary, config, restart, @@ -1180,6 +1166,47 @@ pub fn protect_config( protect_config_platform(config, data_dir, user, relocated) } +/// Starts a service that [`register`] or [`register_rust`] has just registered, once +/// [`grant_service_access`] has let its account read the config. +/// +/// A no-op on Linux, where registration starts the unit itself: the service user was known before +/// the config existed, so `protect_config` had already handed the file to it. On Windows the order +/// is the whole point — see `register_windows_named`. The outcome's `state` is refreshed so the +/// run reports what the service is doing now, not what it was doing before it was started. +pub fn start_registered(outcome: &mut Outcome, binary: &Path, config: &Path) -> Result<()> { + start_registered_platform(outcome, binary, config) +} + +#[cfg(unix)] +fn start_registered_platform(_outcome: &mut Outcome, _binary: &Path, _config: &Path) -> Result<()> { + Ok(()) +} + +#[cfg(windows)] +fn start_registered_platform(outcome: &mut Outcome, binary: &Path, config: &Path) -> Result<()> { + let Outcome::Registered { name, state, .. } = outcome else { + return Ok(()); + }; + let min_sidecar = if name.as_str() == WINDOWS_SERVICE { + MIN_SERVICE_SIDECAR + } else { + MIN_RUST_SERVICE_SIDECAR + }; + // 1056 is ERROR_SERVICE_ALREADY_RUNNING, which is the desired end state, not a failure. + let start = run("sc.exe", &["start", name])?; + if !start.status.success() && start.status.code() != Some(1056) { + anyhow::bail!(windows_start_failure_for( + name, + min_sidecar, + start.status.code().unwrap_or(-1), + binary, + config + )); + } + *state = format!("{}, automatic start", windows_service_state(name)); + Ok(()) +} + /// Grants the registered service account access to what it must read and write. /// /// A no-op on Linux, where the account was known before the config existed and `protect_config`