fix: self update --status and --recover refuse another spelling of the same OCM_HOME or HOME

src/cli/self_update_transaction.rs

@@ -14,7 +14,7 @@ use super::Cli;
 use super::self_cmd::{StagedBinary, validate_staged_binary};
 use crate::infra::download::file_sha256;
 use crate::service::wait_for_gateway_readiness;
-use crate::store::{resolve_ocm_home, resolve_user_home};
+use crate::store::{resolve_ocm_home, resolve_user_home, same_ocm_home};
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
@@ -296,23 +296,35 @@ fn spawn_worker(cli: &Cli, previous: &Path, receipt: &Receipt) -> Result<Child,
     }
     #[cfg(unix)]
     {
-        let mut command = worker_command(previous, receipt)?;
-        command
-            .env_clear()
-            .envs(&cli.env)
-            .env_remove("OCM_ACTIVE_ENV")
-            .env_remove("OPENCLAW_SERVICE_KIND")
-            .env("OCM_SERVICE_EXECUTABLE", &receipt.binary_path)
-            .current_dir(&cli.cwd)
-            .stdin(Stdio::null())
-            .stdout(Stdio::null())
-            .stderr(Stdio::null());
-        command.spawn().map_err(|e| {
-            format!("cannot detach self-update helper; no executable was replaced: {e}")
-        })
+        worker_process(cli, previous, receipt)?
+            .spawn()
+            .map_err(|e| {
+                format!("cannot detach self-update helper; no executable was replaced: {e}")
+            })
     }
 }
 
+#[cfg(unix)]
+fn worker_process(cli: &Cli, previous: &Path, receipt: &Receipt) -> Result<Command, String> {
+    let mut command = worker_command(previous, receipt)?;
+    // The caller may reach the store through another spelling. The worker is
+    // the retained previous release, which may compare the receipt's
+    // directories as text, so it gets the spellings the receipt recorded.
+    command
+        .env_clear()
+        .envs(&cli.env)
+        .env_remove("OCM_ACTIVE_ENV")
+        .env_remove("OPENCLAW_SERVICE_KIND")
+        .env("OCM_HOME", &receipt.ocm_home)
+        .env("HOME", &receipt.user_home)
+        .env("OCM_SERVICE_EXECUTABLE", &receipt.binary_path)
+        .current_dir(&cli.cwd)
+        .stdin(Stdio::null())
+        .stdout(Stdio::null())
+        .stderr(Stdio::null());
+    Ok(command)
+}
+
 #[cfg(unix)]
 fn worker_command(previous: &Path, receipt: &Receipt) -> Result<Command, String> {
     let mut command =
@@ -355,9 +367,18 @@ impl Cli {
         transaction.start_and_wait(self, receipt)
     }
 
+    /// Another spelling of the recorded store or HOME, such as a symlink or
+    /// `/tmp` for `/private/tmp`, is the same store; see `same_ocm_home`.
     fn validate_receipt_home(&self, receipt: &Receipt) -> Result<(), String> {
-        if resolve_ocm_home(&self.env, &self.cwd)? != receipt.ocm_home
-            || resolve_user_home(&self.env) != receipt.user_home
+        let same = |recorded: &Path, current: &Path| {
+            recorded == current
+                || matches!(
+                    (recorded.to_str(), current.to_str()),
+                    (Some(recorded), Some(current)) if same_ocm_home(recorded, current)
+                )
+        };
+        if !same(&receipt.ocm_home, &resolve_ocm_home(&self.env, &self.cwd)?)
+            || !same(&receipt.user_home, &resolve_user_home(&self.env))
         {
             return Err(
                 "self-update belongs to a different HOME or OCM_HOME; use the original store"
@@ -564,3 +585,53 @@ impl Cli {
         Ok(())
     }
 }
+
+#[cfg(all(test, unix))]
+mod tests {
+    use std::collections::BTreeMap;
+    use std::ffi::OsStr;
+    use std::path::{Path, PathBuf};
+
+    use super::{Cli, Phase, Receipt, worker_process};
+
+    #[test]
+    fn worker_runs_with_the_recorded_store_and_home() {
+        let receipt = Receipt {
+            schema: 1,
+            id: "1-1".into(),
+            phase: Phase::RollingBack,
+            binary_path: PathBuf::from("/opt/bin/ocm"),
+            ocm_home: PathBuf::from("/recorded/store"),
+            user_home: PathBuf::from("/recorded/home"),
+            current_version: "1.0.0".into(),
+            target_version: "1.0.1".into(),
+            previous_sha256: String::new(),
+            candidate_sha256: String::new(),
+            daemon_was_running: false,
+            gateways: Vec::new(),
+            error: None,
+            rollback_error: None,
+        };
+        let cli = Cli {
+            env: BTreeMap::from([
+                ("OCM_HOME".into(), "/alias/store".into()),
+                ("HOME".into(), "/alias/home".into()),
+                ("OCM_ACTIVE_ENV".into(), "demo".into()),
+            ]),
+            cwd: PathBuf::from("/caller"),
+        };
+        let command = worker_process(&cli, Path::new("/opt/bin/previous"), &receipt).unwrap();
+        let envs: BTreeMap<&OsStr, Option<&OsStr>> = command.get_envs().collect();
+        // An older retained worker compares these as text, so the caller's
+        // spelling of the same directories must not reach it.
+        assert_eq!(
+            envs[OsStr::new("OCM_HOME")],
+            Some(OsStr::new("/recorded/store"))
+        );
+        assert_eq!(envs[OsStr::new("HOME")], Some(OsStr::new("/recorded/home")));
+        assert!(!matches!(
+            envs.get(OsStr::new("OCM_ACTIVE_ENV")),
+            Some(Some(_))
+        ));
+    }
+}

src/store/layout.rs

@@ -144,7 +144,8 @@ pub fn resolve_ocm_home(env: &BTreeMap<String, String>, cwd: &Path) -> Result<Pa
 /// absolute spellings resolve to the same existing directory, such as a
 /// symlinked store or HOME. A relative, missing or unresolvable recorded store
 /// never matches another spelling, so the caller's working directory is never
-/// part of the decision.
+/// part of the decision. The self-update receipt applies the same rule to its
+/// recorded HOME.
 pub(crate) fn same_ocm_home(recorded: &str, current: &str) -> bool {
     recorded == current
         || (Path::new(recorded).is_absolute()

tests/self_update_transaction_tests.rs

@@ -678,27 +678,90 @@ fn local_transaction_survives_gateway_teardown_and_recovers_failures() {
         stderr(&retry)
     );
     let before_recovery = fs::read(crash.root.child("manager.log")).unwrap();
-    for key in ["HOME", "OCM_HOME"] {
-        let mut foreign = crash.env.clone();
-        foreign.insert(
-            key.into(),
-            crash.root.child("foreign").to_string_lossy().into(),
-        );
-        let rejected = run_ocm_binary(
+    // A missing directory and a different existing one are other stores.
+    fs::create_dir_all(crash.root.child("foreign-existing")).unwrap();
+    for foreign_dir in ["foreign", "foreign-existing"] {
+        for key in ["HOME", "OCM_HOME"] {
+            let mut foreign = crash.env.clone();
+            foreign.insert(
+                key.into(),
+                crash.root.child(foreign_dir).to_string_lossy().into(),
+            );
+            let rejected = run_ocm_binary(
+                &crash.binary,
+                crash.root.path(),
+                &foreign,
+                &["self", "update", "--recover"],
+            );
+            assert!(!rejected.status.success());
+            assert!(stderr(&rejected).contains("different HOME or OCM_HOME"));
+            assert_eq!(crash.receipt()["phase"], "applying");
+            assert_eq!(
+                fs::read(crash.root.child("manager.log")).unwrap(),
+                before_recovery
+            );
+        }
+    }
+    // Other spellings of the recorded store and HOME name the same update:
+    // a symlinked store or HOME, and the absolute path of the relative
+    // OCM_HOME the update started with.
+    let home = PathBuf::from(&crash.env["HOME"]);
+    fs::create_dir_all(&home).unwrap();
+    std::os::unix::fs::symlink(crash.root.child("ocm-home"), crash.root.child("store-link"))
+        .unwrap();
+    std::os::unix::fs::symlink(&home, crash.root.child("home-link")).unwrap();
+    let aliases = [
+        ("OCM_HOME", crash.root.child("store-link")),
+        ("OCM_HOME", crash.root.child("ocm-home")),
+        ("HOME", crash.root.child("home-link")),
+    ];
+    for (key, alias) in &aliases {
+        let mut env = crash.env.clone();
+        env.insert((*key).into(), alias.to_string_lossy().into());
+        let status = run_ocm_binary(
             &crash.binary,
             crash.root.path(),
-            &foreign,
-            &["self", "update", "--recover"],
+            &env,
+            &["self", "update", "--status"],
         );
-        assert!(!rejected.status.success());
-        assert!(stderr(&rejected).contains("different HOME or OCM_HOME"));
-        assert_eq!(crash.receipt()["phase"], "applying");
-        assert_eq!(
-            fs::read(crash.root.child("manager.log")).unwrap(),
-            before_recovery
+        assert!(
+            status.status.success(),
+            "--status through {key}={}: {}",
+            alias.display(),
+            stderr(&status)
         );
+        let shown: Value = serde_json::from_slice(&status.stdout).unwrap();
+        assert_eq!(shown["phase"], "applying");
+        assert_eq!(shown["id"], id);
     }
-    let recovered = crash.run(&["self", "update", "--recover"]);
+    assert_eq!(
+        fs::read(crash.root.child("manager.log")).unwrap(),
+        before_recovery
+    );
+    let mut alias_env = crash.env.clone();
+    alias_env.insert(
+        "OCM_HOME".into(),
+        crash.root.child("store-link").to_string_lossy().into(),
+    );
+    alias_env.insert(
+        "HOME".into(),
+        crash.root.child("home-link").to_string_lossy().into(),
+    );
+    let recovered = run_ocm_binary(
+        &crash.binary,
+        crash.root.path(),
+        &alias_env,
+        &["self", "update", "--recover"],
+    );
+    // The rollback worker is the retained previous release, which may compare
+    // the receipt's directories as text, so it must run with the recorded
+    // spellings. Its service definition path is derived from HOME.
+    let recovery_log = fs::read_to_string(crash.root.child("manager.log")).unwrap();
+    let recovery_log = &recovery_log[before_recovery.len()..];
+    assert!(
+        recovery_log.contains("bootstrap") && !recovery_log.contains("home-link"),
+        "rollback worker did not use the recorded HOME: {recovery_log}"
+    );
     assert_eq!(
         crash.receipt()["phase"],
         "rolledBack",
@@ -712,6 +775,7 @@ fn local_transaction_survives_gateway_teardown_and_recovers_failures() {
     );
     assert_eq!(file_sha256(&crash.binary).unwrap(), original);
     assert!(!crash.root.child("ocm-home/ocm-home").exists());
+    assert!(!crash.root.child("ocm-home/store-link").exists());
     let before_retry = fs::read(crash.root.child("manager.log")).unwrap();
     let _ = crash.run(&["self", "update", "--recover"]);
     assert_eq!(