fix(installer): keep user_created sticky across re-runs
Found on the first real systemd host this installer has ever run on: a container with systemd as PID 1, installing into /usr/bin, /etc and /var/lib for real. `service::prepare` answers "did THIS run create the service account", which is false on every run after the first — by then the account exists. Recording that verbatim made the field describe the run rather than the state, with two consequences: - `install.json` changed on an otherwise-identical second run, breaking the Phase 1 promise that a re-run with nothing new to do writes nothing. - `uninstall` removes only an account it created, so after any second `install` it silently left behind the very user this tool had added. Reproduced before the fix: "left the runicgateway account alone — this installer did not create it", on a host where it plainly had. The record now inherits `true` from a prior record naming the same account, and only that account: inheriting across a rename would authorize deleting a user this installer never made. Not visible on Windows, where the SCM's virtual account is never created by us and goes with the service — which is why three phases of Windows smoke runs never showed it. Verified after the fix on the same host: fresh install records user_created true, an identical second run leaves install.json byte-identical, and uninstall then removes the account, the unit, the service and the binary — leaving sidecar.toml, the database and all 24 overlay files in the ServUO tree exactly where they were. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -249,7 +249,14 @@ pub fn deploy(cli: &Cli, mode: Mode) -> Result<()> {
|
||||
)?;
|
||||
|
||||
// ── The sidecar and its service ──────────────────────────────────────────
|
||||
let sidecar = install_sidecar(cli, &bundle, &sidecar_asset, &layout, scratch.path())?;
|
||||
let sidecar = install_sidecar(
|
||||
cli,
|
||||
&bundle,
|
||||
&sidecar_asset,
|
||||
&layout,
|
||||
scratch.path(),
|
||||
prior.as_ref(),
|
||||
)?;
|
||||
|
||||
// ── Record ───────────────────────────────────────────────────────────────
|
||||
let record = build_record(
|
||||
@@ -415,6 +422,7 @@ fn install_sidecar(
|
||||
asset: &bundle::Asset,
|
||||
layout: &paths::Layout,
|
||||
scratch: &Path,
|
||||
prior: Option<&InstallRecord>,
|
||||
) -> Result<Option<SidecarOutcome>> {
|
||||
ui::heading("uo-link sidecar");
|
||||
|
||||
@@ -539,7 +547,7 @@ fn install_sidecar(
|
||||
name: name.clone(),
|
||||
unit_path: unit_path.as_ref().map(|p| p.display().to_string()),
|
||||
user: user.clone(),
|
||||
user_created: *user_created,
|
||||
user_created: *user_created || created_by_an_earlier_run(prior, user.as_deref()),
|
||||
})
|
||||
}
|
||||
service::Outcome::Skipped { reason, manual } => {
|
||||
@@ -572,6 +580,29 @@ fn install_sidecar(
|
||||
}))
|
||||
}
|
||||
|
||||
/// Whether an earlier run of this installer created the service account.
|
||||
///
|
||||
/// **`user_created` has to be sticky, and this is why.** `service::prepare` answers "did *this run*
|
||||
/// create the account", which is `false` on every run after the first — the account exists by then.
|
||||
/// Recording that verbatim makes the field describe the run instead of the state, with two
|
||||
/// consequences: `install.json` changes on an otherwise-identical second run (breaking Phase 1's
|
||||
/// "a second run writes nothing"), and `uninstall` — which removes only an account it created —
|
||||
/// silently leaves behind the very account this tool added. Both were caught on the first real
|
||||
/// systemd host the installer ever ran on, and neither is visible on Windows, where the SCM's
|
||||
/// virtual account is never "created" by us at all.
|
||||
///
|
||||
/// Matched on the account *name*: a record naming a different user describes a different account,
|
||||
/// and inheriting `true` from it would authorize deleting one this installer never made.
|
||||
fn created_by_an_earlier_run(prior: Option<&InstallRecord>, user: Option<&str>) -> bool {
|
||||
let (Some(prior), Some(user)) = (prior, user) else {
|
||||
return false;
|
||||
};
|
||||
prior
|
||||
.link_record()
|
||||
.and_then(|link| link.service)
|
||||
.is_some_and(|service| service.user.as_deref() == Some(user) && service.user_created)
|
||||
}
|
||||
|
||||
/// Fails the run before it writes anything if this process cannot write where it must.
|
||||
///
|
||||
/// `create_dir_all` succeeding is not the same question as "can this process write here" — it
|
||||
@@ -759,6 +790,61 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn an_account_this_installer_created_stays_recorded_as_created() {
|
||||
// Caught on the first real systemd host: `prepare` reports "did THIS run create it", which
|
||||
// is false from the second run on. Recording that verbatim rewrote install.json on an
|
||||
// identical re-run and, worse, left `uninstall` believing the account was somebody else's.
|
||||
let mut record = record_for("/opt/ServUO");
|
||||
let service = |user: &str, created: bool| ServiceRecord {
|
||||
kind: "systemd".into(),
|
||||
name: "runicgateway-link.service".into(),
|
||||
unit_path: Some("/etc/systemd/system/runicgateway-link.service".into()),
|
||||
user: Some(user.into()),
|
||||
user_created: created,
|
||||
};
|
||||
let with_service = |record: &mut InstallRecord, service: ServiceRecord| {
|
||||
record.link = serde_json::to_value(LinkRecord {
|
||||
repo: "RunicGateway/link".into(),
|
||||
tag: "v1.1.0".into(),
|
||||
version: "1.1.0".into(),
|
||||
protocol: 3,
|
||||
binary: BinaryRef {
|
||||
path: "/usr/bin/runicgateway-link".into(),
|
||||
sha256: "aa".into(),
|
||||
},
|
||||
config_path: "/etc/runicgateway/sidecar.toml".into(),
|
||||
db_path: "/var/lib/runicgateway/uo-link.db".into(),
|
||||
service: Some(service),
|
||||
})
|
||||
.ok();
|
||||
};
|
||||
|
||||
with_service(&mut record, service("runicgateway", true));
|
||||
assert!(created_by_an_earlier_run(
|
||||
Some(&record),
|
||||
Some("runicgateway")
|
||||
));
|
||||
|
||||
// An account this installer found already there stays somebody else's, forever.
|
||||
with_service(&mut record, service("runicgateway", false));
|
||||
assert!(!created_by_an_earlier_run(
|
||||
Some(&record),
|
||||
Some("runicgateway")
|
||||
));
|
||||
|
||||
// A record naming a different account says nothing about this one — inheriting `true`
|
||||
// there would authorize deleting a user this installer never made.
|
||||
with_service(&mut record, service("someone-else", true));
|
||||
assert!(!created_by_an_earlier_run(
|
||||
Some(&record),
|
||||
Some("runicgateway")
|
||||
));
|
||||
|
||||
assert!(!created_by_an_earlier_run(None, Some("runicgateway")));
|
||||
assert!(!created_by_an_earlier_run(Some(&record), None));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_record_for_this_tree_is_used() {
|
||||
let record = record_for("/opt/ServUO");
|
||||
|
||||
Reference in New Issue
Block a user