From 9895695461c199bc1aa5f92ff8867a84e6140c8e Mon Sep 17 00:00:00 2001 From: Shakker Date: Mon, 2 Nov 2026 11:32:49 +0000 Subject: [PATCH 1/8] fix: worker folders, list-entry includes and placeholder ~/ paths point under the env root after migrate `ocm migrate` and `ocm adopt import` now also keep `~/...` values in `nodeHost.workerRuns.folders`, in includes placed at a list position and in values containing a `${VAR}` placeholder pointing at the location the plain install used. Home-relative trust roots in `skills.load.allowSymlinkTargets` and `security.installPolicy.exec.trustedDirs` stay as written, and migration now warns that they resolve under the new environment root. Fixes #972. --- CHANGELOG.md | 1 + docs/USAGE.md | 23 +- src/cli/migrate.rs | 16 ++ src/migrate.rs | 5 + src/store/openclaw_config.rs | 408 +++++++++++++++++++++++-------- src/store/openclaw_workspaces.rs | 49 ++-- tests/migrate_command_tests.rs | 239 +++++++++++++++++- 7 files changed, 606 insertions(+), 135 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e10a7056..d58b177d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ All notable changes to OCM are documented here. - Find the installed npm launcher by default when building and installing runtimes on Windows. - Remove the hidden staging file and SQLite snapshot copies when `ocm env export` is stopped by SIGINT, SIGTERM or SIGHUP, and exit with status 130. Signals ignored at startup, as under `nohup`, stay ignored. - Keep `~/...` config paths outside `~/.openclaw`, such as local plugin, extra skills, agent directory and session or cron store paths, pointing at the same location after `migrate` and `adopt import`, instead of letting them resolve under the new environment root. Other config text is left unchanged. +- Also keep `~/...` values in `nodeHost.workerRuns.folders`, in includes placed at a list position and in values containing a `${VAR}` placeholder pointing at the same location after `migrate` and `adopt import`. Home-relative trust roots in `skills.load.allowSymlinkTargets` and `security.installPolicy.exec.trustedDirs` stay as written, and migration now warns that they resolve under the new environment root. ## 0.3.8 - 2026-10-29 diff --git a/docs/USAGE.md b/docs/USAGE.md index c75876a3..306dc96c 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -1288,14 +1288,21 @@ including `skills.load.extraDirs`, `hooks.internal.load.extraDirs`, agent `cwd`, handling and lexical `.` and `..` resolution as its OpenClaw consumer. Settings OpenClaw expands against the OS home keep their meaning without a rewrite, because the environment keeps your `HOME`. Other strings, such as message text, -stay as written. Not rewritten yet -([#972](https://github.com/openclaw/ocm/issues/972)): values containing a -`${VAR}` placeholder, which OpenClaw substitutes before resolving the path; -`nodeHost.workerRuns.folders`; values in an include placed at a list position; -and the trust roots `skills.load.allowSymlinkTargets` and -`security.installPolicy.exec.trustedDirs`, which keep failing closed. The source -home is your home directory when the migrated directory is your own -`~/.openclaw`, through any path that leads to it; +stay as written. Path values in an include placed at a list position are +rewritten like any other. In a value containing a `${VAR}` placeholder, only the directories before the +first placeholder are resolved; OpenClaw substitutes the placeholder and +resolves the rest, as it would for the same value spelled from your home, so a +placeholder that itself leads into `~/.openclaw` names the source's state +directory. `nodeHost.workerRuns.folders` entries, which OpenClaw resolves +through symlinks, only have their leading `~` replaced, so a link before `..` +still decides where they lead. The trust roots `skills.load.allowSymlinkTargets` +and `security.installPolicy.exec.trustedDirs` are not rewritten, because that +would change what the new environment trusts: their `~/...` entries outside +`~/.openclaw` now resolve under the environment root, and migration prints a +warning naming each such setting. Replace those entries with absolute paths to +keep trusting the original directories. The source home is your home +directory when the migrated directory is your own `~/.openclaw`, through any +path that leads to it; otherwise it is the directory that contains the migrated `.openclaw` as you spelled it, or as it is stored when only that path is named `.openclaw`, or your home directory for a state directory with another name. Values under diff --git a/src/cli/migrate.rs b/src/cli/migrate.rs index bf81bc3b..a5a14f97 100644 --- a/src/cli/migrate.rs +++ b/src/cli/migrate.rs @@ -169,6 +169,10 @@ impl Cli { result.sandbox_port, ); self.warn_external_migration_plugins(&result.external_plugin_ids); + self.warn_kept_home_relative_trust_roots( + &result.summary.name, + &result.kept_home_relative_trust_roots, + ); let summary = result.summary; if json_flag { @@ -211,6 +215,10 @@ impl Cli { result.sandbox_port, ); self.warn_external_migration_plugins(&result.external_plugin_ids); + self.warn_kept_home_relative_trust_roots( + &result.summary.name, + &result.kept_home_relative_trust_roots, + ); let summary = result.summary; if json_flag { @@ -243,6 +251,14 @@ impl Cli { } } + fn warn_kept_home_relative_trust_roots(&self, env_name: &str, settings: &[String]) { + for setting in settings { + self.stderr_line(format!( + "warning: env {env_name} keeps the ~/ entries of trust root {setting} as written; they now resolve under the env root instead of the source home, so OCM did not carry that trust over. Replace them with absolute paths to keep trusting the original directories" + )); + } + } + fn handle_adopt_inspect(&self, args: Vec) -> Result { let (args, json_flag, profile) = self.consume_human_output_flags(args, "adopt inspect")?; if args.len() > 1 { diff --git a/src/migrate.rs b/src/migrate.rs index 3a9ee3f8..d3e44dcb 100644 --- a/src/migrate.rs +++ b/src/migrate.rs @@ -50,6 +50,7 @@ pub(crate) struct MigrateHomeResult { pub cleared_sandbox_origin: bool, pub sandbox_port: Option, pub external_plugin_ids: Vec, + pub kept_home_relative_trust_roots: Vec, } #[derive(Clone, Debug)] @@ -353,6 +354,7 @@ struct CompletedMigrationImport { cleared_sandbox_origin: bool, sandbox_port: Option, external_plugin_ids: Vec, + kept_home_relative_trust_roots: Vec, } pub fn default_migration_source_home(env: &BTreeMap) -> PathBuf { @@ -518,6 +520,7 @@ fn migrate_plain_openclaw_home_inner( cleared_sandbox_origin: completed.cleared_sandbox_origin, sandbox_port: completed.sandbox_port, external_plugin_ids: completed.external_plugin_ids, + kept_home_relative_trust_roots: completed.kept_home_relative_trust_roots, }) } @@ -646,6 +649,7 @@ fn complete_migration_import( cleared_sandbox_origin: config_rewrite.cleared_sandbox_origin, sandbox_port: config_rewrite.sandbox_port, external_plugin_ids: runtime_state.external_plugin_ids, + kept_home_relative_trust_roots: config_rewrite.kept_home_relative_trust_roots, }); }; @@ -671,6 +675,7 @@ fn complete_migration_import( cleared_sandbox_origin: config_rewrite.cleared_sandbox_origin, sandbox_port: config_rewrite.sandbox_port, external_plugin_ids: runtime_state.external_plugin_ids, + kept_home_relative_trust_roots: config_rewrite.kept_home_relative_trust_roots, }), Err(error) => { let rollback = rollback_migrated_launcher(created_launcher.as_deref(), env, cwd); diff --git a/src/store/openclaw_config.rs b/src/store/openclaw_config.rs index 45fafd22..5f3b9894 100644 --- a/src/store/openclaw_config.rs +++ b/src/store/openclaw_config.rs @@ -15,7 +15,7 @@ use super::common::{ensure_dir, path_exists, write_file_replacing_path}; use super::gateway_ports::read_port_number; use super::layout::{EnvPaths, clean_path, derive_env_paths, display_path, resolve_user_home}; use super::openclaw_workspaces::{ - OpenClawConfigReadContext, OpenClawConfigReadError, OpenClawWorkspaceInventory, + ConfigSegment, OpenClawConfigReadContext, OpenClawConfigReadError, OpenClawWorkspaceInventory, OpenClawWorkspaceRuntime, load_effective_openclaw_config, read_openclaw_config_field, read_regular_text_file, resolve_effective_openclaw_config_value, resolve_openclaw_config_env, }; @@ -38,6 +38,9 @@ pub(crate) struct OpenClawConfigAudit { pub(crate) struct OpenClawConfigRewriteOutcome { pub cleared_sandbox_origin: bool, pub sandbox_port: Option, + /// Trust-root settings whose home-relative entries migration kept as + /// written, so they now resolve under the environment root. + pub kept_home_relative_trust_roots: Vec, } pub(crate) fn audit_openclaw_config( @@ -211,18 +214,20 @@ pub(crate) fn rewrite_openclaw_config_for_migration( gateway_port: Option, sandbox_origin: Option<&str>, ) -> Result { - expand_migrated_home_relative_paths(target_paths, source_home)?; + let kept_trust_roots = expand_migrated_home_relative_paths(target_paths, source_home)?; rewrite_migrated_plugin_load_paths( target_paths, source_state_root, canonical_source_state_root, )?; - rewrite_openclaw_config_with_root_mapping( + let mut outcome = rewrite_openclaw_config_with_root_mapping( target_paths, Some((source_state_root, target_paths.state_dir.as_path())), gateway_port, SandboxOriginPolicy::NewEnvironment(sandbox_origin), - ) + )?; + outcome.kept_home_relative_trust_roots = kept_trust_roots.into_iter().collect(); + Ok(outcome) } /// A plain install expands `~` against its home, but the environment runs @@ -231,15 +236,18 @@ pub(crate) fn rewrite_openclaw_config_for_migration( /// directory; every other home-relative path setting is spelled out against /// the source home so it keeps naming the same location. Absolute values /// inside the source state directory are then relocated by the later rewrites. +/// Returns the trust-root settings whose home-relative entries were kept as +/// written. fn expand_migrated_home_relative_paths( target: &EnvPaths, source_home: &Path, -) -> Result<(), String> { +) -> Result, String> { + let mut kept_trust_roots = BTreeSet::new(); if !source_home.is_absolute() { - return Ok(()); + return Ok(kept_trust_roots); } let Some(effective) = load_effective_openclaw_config(&target.config_path)? else { - return Ok(()); + return Ok(kept_trust_roots); }; let mut scopes = effective.include_scopes; scopes.insert(target.config_path.clone(), BTreeSet::from([Vec::new()])); @@ -248,13 +256,14 @@ fn expand_migrated_home_relative_paths( continue; }; let mut rewritten = None; - for scope in scopes { - // Include scopes are typed as object keys: a recorded index cannot - // be told apart from a digit-only key, so a value in an include - // placed at a list position keeps its spelling (openclaw/ocm#972). - let mut scope = scope.into_iter().map(ConfigSegment::Key).collect(); + for mut scope in scopes { let mut candidate = value.clone(); - expand_home_relative_paths_in_scope(&mut candidate, &mut scope, source_home); + expand_home_relative_paths_in_scope( + &mut candidate, + &mut scope, + source_home, + &mut kept_trust_roots, + ); if rewritten .as_ref() .is_some_and(|previous| previous != &candidate) @@ -270,29 +279,34 @@ fn expand_migrated_home_relative_paths( write_copied_config_value(&path, &target.state_dir, &rewritten)?; } } - Ok(()) -} - -/// One step from the config root to a value: an object key or a list entry. -#[derive(Clone, Debug, PartialEq, Eq)] -enum ConfigSegment { - Key(String), - Entry, + Ok(kept_trust_roots) } /// Expands the home-relative path settings in a config fragment included at -/// `scope`. `$include` directives keep their own relocation. +/// `scope`, and records the trust-root settings it keeps as written. +/// `$include` directives keep their own relocation. fn expand_home_relative_paths_in_scope( value: &mut Value, scope: &mut Vec, home: &Path, + kept_trust_roots: &mut BTreeSet, ) -> bool { match value { Value::String(raw) => { - let Some(trims) = home_relative_path_setting(scope) else { - return false; + let expanded = match home_relative_path_setting(scope) { + Some(HomePathRule::Lexical { trims }) => { + expand_home_relative_path(raw, home, trims) + } + Some(HomePathRule::Physical) => expand_physical_home_relative_path(raw, home), + Some(HomePathRule::TrustRoot(setting)) => { + if expand_home_relative_path(raw, home, true).is_some() { + kept_trust_roots.insert(setting.to_string()); + } + None + } + None => None, }; - let Some(expanded) = expand_home_relative_path(raw, home, trims) else { + let Some(expanded) = expanded else { return false; }; *raw = expanded; @@ -300,9 +314,10 @@ fn expand_home_relative_paths_in_scope( } Value::Array(values) => { let mut changed = false; - for nested in values { - scope.push(ConfigSegment::Entry); - changed |= expand_home_relative_paths_in_scope(nested, scope, home); + for (index, nested) in values.iter_mut().enumerate() { + scope.push(ConfigSegment::Index(index)); + changed |= + expand_home_relative_paths_in_scope(nested, scope, home, kept_trust_roots); scope.pop(); } changed @@ -311,7 +326,8 @@ fn expand_home_relative_paths_in_scope( let mut changed = false; for (key, nested) in values.iter_mut().filter(|(key, _)| *key != "$include") { scope.push(ConfigSegment::Key(key.clone())); - changed |= expand_home_relative_paths_in_scope(nested, scope, home); + changed |= + expand_home_relative_paths_in_scope(nested, scope, home, kept_trust_roots); scope.pop(); } changed @@ -320,53 +336,96 @@ fn expand_home_relative_paths_in_scope( } } +/// How OpenClaw resolves a home-relative setting. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum HomePathRule { + /// The leading `~` is replaced, after trimming when `trims` is set, and + /// the path is resolved lexically. + Lexical { trims: bool }, + /// The leading `~` is replaced and the path is resolved through links + /// (`realpath`), so a link before `..` decides where it leads. + Physical, + /// A trust root, named here. Spelling it out would change what the + /// environment trusts, so it stays as written and is reported. + TrustRoot(&'static str), +} + /// Settings whose own OpenClaw consumers expand `~` against `OPENCLAW_HOME` /// with OpenClaw's shared resolver although its runtime config rule does -/// not, with whether the consumer trims the value first. `*` stands for any -/// one object key and `[]` for a list entry. Derived from the consumers at -/// OpenClaw a7fe06b17a9. Settings their consumers expand against the OS home -/// (string `memory.search.extraPaths` entries, `messages.usageTemplate`, the +/// not, with how each resolves it. `*` stands for any one object key and +/// `[]` for a list entry. Derived from the consumers at OpenClaw +/// a7fe06b17a9. Settings their consumers expand against the OS home (string +/// `memory.search.extraPaths` entries, `messages.usageTemplate`, the /// voice-call store and the file-transfer path lists) keep their meaning, -/// because `HOME` is unchanged in the environment. Not rewritten yet -/// (openclaw/ocm#972): `nodeHost.workerRuns.folders`, whose consumer resolves -/// links before `..`, unlike the lexical expansion used here, and the trust -/// roots `skills.load.allowSymlinkTargets` and -/// `security.installPolicy.exec.trustedDirs`, which keep failing closed. -const CONSUMER_HOME_PATH_SETTINGS: &[(&[&str], bool)] = &[ - // src/skills/loading/workspace-skill-sources.ts, symlink-targets.ts - (&["skills", "load", "extraDirs", "[]"], true), +/// because `HOME` is unchanged in the environment. +const CONSUMER_HOME_PATH_SETTINGS: &[(&[&str], HomePathRule)] = &[ + // src/skills/loading/workspace-skill-sources.ts + ( + &["skills", "load", "extraDirs", "[]"], + HomePathRule::Lexical { trims: true }, + ), // src/hooks/workspace.ts - (&["hooks", "internal", "load", "extraDirs", "[]"], true), + ( + &["hooks", "internal", "load", "extraDirs", "[]"], + HomePathRule::Lexical { trims: true }, + ), // src/config/sessions/paths.ts expands `~` without trimming first. - (&["session", "store"], false), + ( + &["session", "store"], + HomePathRule::Lexical { trims: false }, + ), // src/cron/store/paths.ts - (&["cron", "store"], true), + (&["cron", "store"], HomePathRule::Lexical { trims: true }), // src/agents/agent-scope-config.ts - (&["agents", "defaults", "cwd"], true), - (&["agents", "entries", "*", "cwd"], true), - (&["agents", "list", "[]", "cwd"], true), + ( + &["agents", "defaults", "cwd"], + HomePathRule::Lexical { trims: true }, + ), + ( + &["agents", "entries", "*", "cwd"], + HomePathRule::Lexical { trims: true }, + ), + ( + &["agents", "list", "[]", "cwd"], + HomePathRule::Lexical { trims: true }, + ), + // src/node-host/node-worker-device-folder.ts: `expandHomePrefix`, then + // `fs.realpathSync.native`, without trimming. + ( + &["nodeHost", "workerRuns", "folders", "[]"], + HomePathRule::Physical, + ), + // src/skills/loading/symlink-targets.ts and src/security/install-policy.ts + ( + &["skills", "load", "allowSymlinkTargets", "[]"], + HomePathRule::TrustRoot("skills.load.allowSymlinkTargets"), + ), + ( + &["security", "installPolicy", "exec", "trustedDirs", "[]"], + HomePathRule::TrustRoot("security.installPolicy.exec.trustedDirs"), + ), ]; -/// Whether OpenClaw expands a leading `~` in the string setting at `scope`, -/// and whether it trims the value first. OpenClaw's runtime config -/// (`materializeRuntimeConfig` in `src/config/materialize.ts`) expands keys -/// ending in `dir`, `path`, `paths`, `file`, `root` or `workspace`, the keys -/// `paths` and `pathPrepend`, and the direct entries of `paths` and -/// `pathPrepend` lists; [`CONSUMER_HOME_PATH_SETTINGS`] lists the settings -/// that are expanded by their own consumers. Other strings, such as message -/// text, stay as written. -fn home_relative_path_setting(scope: &[ConfigSegment]) -> Option { - if let Some((_, trims)) = CONSUMER_HOME_PATH_SETTINGS +/// How OpenClaw resolves a leading `~` in the string setting at `scope`, if +/// it expands one. OpenClaw's runtime config (`materializeRuntimeConfig` in +/// `src/config/materialize.ts`) expands keys ending in `dir`, `path`, +/// `paths`, `file`, `root` or `workspace`, the keys `paths` and +/// `pathPrepend`, and the direct entries of `paths` and `pathPrepend` lists; +/// [`CONSUMER_HOME_PATH_SETTINGS`] lists the settings that are expanded by +/// their own consumers. Other strings, such as message text, stay as written. +fn home_relative_path_setting(scope: &[ConfigSegment]) -> Option { + if let Some((_, rule)) = CONSUMER_HOME_PATH_SETTINGS .iter() .find(|(pattern, _)| config_scope_matches(scope, pattern)) { - return Some(*trims); + return Some(*rule); } const PATH_LISTS: [&str; 2] = ["paths", "pathPrepend"]; const PATH_KEY_SUFFIXES: [&str; 6] = ["dir", "path", "paths", "file", "root", "workspace"]; + let lexical = HomePathRule::Lexical { trims: true }; match scope { - [.., ConfigSegment::Key(list), ConfigSegment::Entry] => { - PATH_LISTS.contains(&list.as_str()).then_some(true) + [.., ConfigSegment::Key(list), ConfigSegment::Index(_)] => { + PATH_LISTS.contains(&list.as_str()).then_some(lexical) } [.., ConfigSegment::Key(key)] => { let lower = key.to_ascii_lowercase(); @@ -374,7 +433,7 @@ fn home_relative_path_setting(scope: &[ConfigSegment]) -> Option { || PATH_KEY_SUFFIXES .iter() .any(|suffix| lower.ends_with(suffix))) - .then_some(true) + .then_some(lexical) } _ => None, } @@ -386,7 +445,7 @@ fn config_scope_matches(scope: &[ConfigSegment], pattern: &[&str]) -> bool { .iter() .zip(pattern) .all(|(segment, expected)| match (segment, *expected) { - (ConfigSegment::Entry, "[]") => true, + (ConfigSegment::Index(_), "[]") => true, (ConfigSegment::Key(_), "*") => true, (ConfigSegment::Key(key), expected) => key == expected, _ => false, @@ -410,30 +469,75 @@ fn is_ecmascript_whitespace(c: char) -> bool { ) } +fn is_home_relative(value: &str) -> bool { + value == "~" || value.starts_with("~/") || value.starts_with("~\\") +} + +/// Splits a home-relative value before the separator that precedes its +/// first `${VAR}` placeholder. OpenClaw substitutes placeholders before it +/// resolves the path, so a placeholder's value can still add `..` or name +/// any directory; only the literal directories before it are known here. +fn split_before_placeholder(value: &str) -> (&str, &str) { + let Some(placeholder) = value.find("${") else { + return (value, ""); + }; + // `~` is followed by a separator, so the split never falls before it. + let split = value[..placeholder] + .rfind(std::path::is_separator) + .unwrap_or(1); + value.split_at(split) +} + fn expand_home_relative_path(raw: &str, home: &Path, trims: bool) -> Option { let value = if trims { raw.trim_matches(is_ecmascript_whitespace) } else { raw }; - if value != "~" && !value.starts_with("~/") && !value.starts_with("~\\") { - return None; - } - // OpenClaw substitutes `${VAR}` before resolving `..`, which the lexical - // expansion here cannot reproduce; leave such values as written for now - // (openclaw/ocm#972). - if value.contains("${") { + if !is_home_relative(value) { return None; } // Expand as OpenClaw does: replace the leading `~`, then resolve `.` and // `..` lexically, so later rewrites cannot follow links OpenClaw ignores. + // A placeholder and what follows it stay as written: OpenClaw resolves + // them after substituting the placeholder, exactly as it resolves the + // same value spelled from the home. + let (literal, mut rest) = split_before_placeholder(value); let mut expanded = home.as_os_str().to_os_string(); - expanded.push(&value[1..]); + expanded.push(&literal[1..]); let expanded = clean_path(Path::new(&expanded)); if expanded.starts_with(home.join(".openclaw")) { return None; } - expanded.to_str().map(str::to_string) + let expanded = expanded.to_str()?; + if expanded.ends_with(std::path::is_separator) { + // The literal part resolved to a filesystem root. + rest = &rest[1..]; + } + Some(format!("{expanded}{rest}")) +} + +/// Expands a setting OpenClaw resolves through links. Only the leading `~` +/// is replaced: the consumer follows each link before the `..` after it, +/// which removing `..` here would change. A value under `~/.openclaw` with +/// no `..` already names the copied state directory and stays as written; +/// the later relocation follows links the same way the consumer does. +fn expand_physical_home_relative_path(raw: &str, home: &Path) -> Option { + if !is_home_relative(raw) { + return None; + } + let (literal, _) = split_before_placeholder(raw); + let mut literal_path = home.as_os_str().to_os_string(); + literal_path.push(&literal[1..]); + let literal_path = Path::new(&literal_path); + if literal_path.starts_with(home.join(".openclaw")) + && !literal_path + .components() + .any(|component| component == std::path::Component::ParentDir) + { + return None; + } + Some(format!("{}{}", home.to_str()?, &raw[1..])) } /// Moves a Gateway port that a copied `$include` supplies to `gateway_port`, @@ -456,7 +560,7 @@ fn rewrite_included_gateway_port( // A file reached through several paths, such as a symlinked include, is // one include: rewriting it changes every place that reads it, so its // scopes are checked together. - let mut includes = BTreeMap::, BTreeSet>)>::new(); + let mut includes = BTreeMap::, BTreeSet>)>::new(); for (path, scopes) in effective.include_scopes { let file = fs::canonicalize(&path).unwrap_or_else(|_| path.clone()); let (paths, file_scopes) = includes.entry(file).or_default(); @@ -511,9 +615,9 @@ fn rewrite_included_gateway_port( } /// Rewrites `gateway.port` in a config fragment included at `scope`. -fn rewrite_gateway_port_in_scope(value: &mut Value, scope: &[String], gateway_port: u32) { +fn rewrite_gateway_port_in_scope(value: &mut Value, scope: &[ConfigSegment], gateway_port: u32) { const GATEWAY_PORT: [&str; 2] = ["gateway", "port"]; - if scope.len() > GATEWAY_PORT.len() || scope.iter().zip(GATEWAY_PORT).any(|(a, b)| a != b) { + if scope.len() > GATEWAY_PORT.len() || scope.iter().zip(GATEWAY_PORT).any(|(a, b)| !a.is(b)) { return; } let mut port = value; @@ -574,13 +678,16 @@ fn rewrite_migrated_plugin_load_paths( fn rewrite_migrated_plugin_paths_in_scope( value: &mut Value, - scope: &mut Vec, + scope: &mut Vec, source: &Path, canonical_source: &Path, target: &Path, ) { if scope.len() == 4 - && scope[..3] == ["plugins", "load", "paths"] + && scope + .iter() + .zip(["plugins", "load", "paths"]) + .all(|(segment, name)| segment.is(name)) && let Some(raw) = value.as_str() { let path = Path::new(raw); @@ -599,7 +706,7 @@ fn rewrite_migrated_plugin_paths_in_scope( match value { Value::Object(object) => { for (key, child) in object { - scope.push(key.clone()); + scope.push(ConfigSegment::Key(key.clone())); rewrite_migrated_plugin_paths_in_scope( child, scope, @@ -612,7 +719,7 @@ fn rewrite_migrated_plugin_paths_in_scope( } Value::Array(array) => { for (index, child) in array.iter_mut().enumerate() { - scope.push(index.to_string()); + scope.push(ConfigSegment::Index(index)); rewrite_migrated_plugin_paths_in_scope( child, scope, @@ -1099,13 +1206,13 @@ pub(crate) fn rewrite_included_agent_directories_for_target( fn rewrite_agent_directory_templates( value: &mut Value, - scope: &mut Vec, + scope: &mut Vec, replacements: &BTreeMap, ) -> bool { if scope.len() == 4 - && scope[0] == "agents" - && matches!(scope[1].as_str(), "entries" | "list") - && scope[3] == "agentDir" + && scope[0].is("agents") + && (scope[1].is("entries") || scope[1].is("list")) + && scope[3].is("agentDir") && let Some(replacement) = value.as_str().and_then(|raw| replacements.get(raw)) { *value = Value::String(replacement.clone()); @@ -1115,14 +1222,14 @@ fn rewrite_agent_directory_templates( match value { Value::Object(object) => { for (key, child) in object { - scope.push(key.clone()); + scope.push(ConfigSegment::Key(key.clone())); changed |= rewrite_agent_directory_templates(child, scope, replacements); scope.pop(); } } Value::Array(array) => { for (index, child) in array.iter_mut().enumerate() { - scope.push(index.to_string()); + scope.push(ConfigSegment::Index(index)); changed |= rewrite_agent_directory_templates(child, scope, replacements); scope.pop(); } @@ -2969,15 +3076,29 @@ mod tests { assert_eq!(expand("~/x/../.openclaw/y"), None); // OpenClaw replaces the `~` of `~\` too; on POSIX the rest stays one name. assert_eq!(expand("~\\dev").as_deref(), Some("/home/alice\\dev")); - for unchanged in [ - "~alice/dev", - "dev/~/p", - "/abs/~/p", - "~~/p", - "~/${UP}/../shared", - ] { + for unchanged in ["~alice/dev", "dev/~/p", "/abs/~/p", "~~/p"] { assert_eq!(expand(unchanged), None, "{unchanged}"); } + // OpenClaw substitutes a placeholder before it resolves `..`, so only + // the literal directories before it are resolved here. + assert_eq!( + expand("~/${UP}/../shared").as_deref(), + Some("/home/alice/${UP}/../shared") + ); + assert_eq!( + expand("~/a/../dev/${SUB}/x").as_deref(), + Some("/home/alice/dev/${SUB}/x") + ); + assert_eq!( + expand("~/dev/p${N}.log").as_deref(), + Some("/home/alice/dev/p${N}.log") + ); + assert_eq!(expand("~/../../${X}").as_deref(), Some("/${X}")); + assert_eq!(expand("~/.openclaw/${SUB}/x"), None); + assert_eq!( + expand("~/.openclaw/../${SUB}").as_deref(), + Some("/home/alice/${SUB}") + ); // A setting that does not trim keeps its whitespace semantics. assert_eq!(expand_home_relative_path(" ~/s.json", home, false), None); assert_eq!( @@ -2992,7 +3113,7 @@ mod tests { let scope = scope .iter() .map(|segment| match *segment { - "[]" => ConfigSegment::Entry, + "[]" => ConfigSegment::Index(0), key => ConfigSegment::Key(key.to_string()), }) .collect::>(); @@ -3015,9 +3136,30 @@ mod tests { &["agents", "defaults", "workspace"], &["cron", "store"], ] { - assert_eq!(setting(path), Some(true), "{path:?}"); + assert_eq!( + setting(path), + Some(HomePathRule::Lexical { trims: true }), + "{path:?}" + ); } - assert_eq!(setting(&["session", "store"]), Some(false)); + assert_eq!( + setting(&["session", "store"]), + Some(HomePathRule::Lexical { trims: false }) + ); + assert_eq!( + setting(&["nodeHost", "workerRuns", "folders", "[]"]), + Some(HomePathRule::Physical) + ); + assert_eq!( + setting(&["skills", "load", "allowSymlinkTargets", "[]"]), + Some(HomePathRule::TrustRoot("skills.load.allowSymlinkTargets")) + ); + assert_eq!( + setting(&["security", "installPolicy", "exec", "trustedDirs", "[]"]), + Some(HomePathRule::TrustRoot( + "security.installPolicy.exec.trustedDirs" + )) + ); for text in [ &["messages", "responsePrefix"][..], &["messages", "usageTemplate"], @@ -3034,23 +3176,91 @@ mod tests { "allowReadPaths", "[]", ], - // Trust roots keep failing closed (openclaw/ocm#972). - &["skills", "load", "allowSymlinkTargets", "[]"], - &["security", "installPolicy", "exec", "trustedDirs", "[]"], - // Resolved through links before `..`. - &["nodeHost", "workerRuns", "folders", "[]"], &["agents", "list", "[]", "identity", "name"], &["plugins", "entries", "demo", "config", "dirs", "[]"], // A digit-only key is an object key, not a list entry. &["plugins", "entries", "demo", "config", "paths", "0"], &["memory", "store"], &["agents", "defaults", "cwd", "[]"], + &["nodeHost", "workerRuns", "folders"], &[], ] { assert_eq!(setting(text), None, "{text:?}"); } } + #[cfg(unix)] + #[test] + fn migration_home_expansion_keeps_link_traversal_for_physical_settings() { + let home = Path::new("/home/alice"); + let expand = |raw| expand_physical_home_relative_path(raw, home); + assert_eq!(expand("~/work").as_deref(), Some("/home/alice/work")); + assert_eq!(expand("~").as_deref(), Some("/home/alice")); + // `..` stays for the consumer to resolve after following `hop`. + assert_eq!( + expand("~/hop/../work").as_deref(), + Some("/home/alice/hop/../work") + ); + assert_eq!( + expand("~/.openclaw/../work").as_deref(), + Some("/home/alice/.openclaw/../work") + ); + assert_eq!( + expand("~/${SUB}/work").as_deref(), + Some("/home/alice/${SUB}/work") + ); + // The consumer does not trim. + assert_eq!(expand(" ~/work"), None); + for copied in ["~/.openclaw", "~/.openclaw/wf", "~/.openclaw/${SUB}"] { + assert_eq!(expand(copied), None, "{copied}"); + } + assert_eq!( + expand("~/.openclawish").as_deref(), + Some("/home/alice/.openclawish") + ); + } + + #[test] + fn migration_home_expansion_types_include_scopes_from_traversal() { + let root = tempfile::tempdir().unwrap(); + let config = root.path().join("openclaw.json"); + // Concatenating `a.json` and `b.json` merges an object with a + // digit-only key and a list into one `paths` value; the include at + // the list entry keeps its list-index segment. + fs::write(&config, r#"{"hooks":{"$include":["./a.json","./b.json"]}}"#).unwrap(); + fs::write( + root.path().join("a.json"), + r#"{"items":[{"paths":{"0":"literal"}}]}"#, + ) + .unwrap(); + fs::write( + root.path().join("b.json"), + r#"{"items":[{"paths":[{"$include":"./p.json"}]}]}"#, + ) + .unwrap(); + fs::write(root.path().join("p.json"), r#""~/bin""#).unwrap(); + let effective = load_effective_openclaw_config(&config).unwrap().unwrap(); + let (_, scopes) = effective + .include_scopes + .iter() + .find(|(path, _)| path.ends_with("p.json")) + .unwrap(); + assert_eq!( + scopes.iter().collect::>(), + [&vec![ + ConfigSegment::Key("hooks".to_string()), + ConfigSegment::Key("items".to_string()), + ConfigSegment::Index(0), + ConfigSegment::Key("paths".to_string()), + ConfigSegment::Index(0), + ]] + ); + assert_eq!( + home_relative_path_setting(scopes.first().unwrap()), + Some(HomePathRule::Lexical { trims: true }) + ); + } + fn meta(name: &str, root: &str, gateway_port: Option) -> EnvMeta { EnvMeta { upgrade_independent_paths: Vec::new(), diff --git a/src/store/openclaw_workspaces.rs b/src/store/openclaw_workspaces.rs index 5abb7769..77b5a120 100644 --- a/src/store/openclaw_workspaces.rs +++ b/src/store/openclaw_workspaces.rs @@ -37,7 +37,26 @@ impl<'a> OpenClawWorkspaceRuntime<'a> { pub(crate) struct EffectiveOpenClawConfig { pub(crate) value: Value, pub(crate) include_paths: BTreeSet, - pub(crate) include_scopes: BTreeMap>>, + pub(crate) include_scopes: BTreeMap>>, +} + +/// One step from the config root to a value, recorded while includes are +/// traversed: once arrays from several includes are concatenated, a list +/// index can no longer be told apart from a digit-only key in the result. +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub(crate) enum ConfigSegment { + Key(String), + Index(usize), +} + +impl ConfigSegment { + /// Whether this step is spelled `name`; an index as its decimal digits. + pub(crate) fn is(&self, name: &str) -> bool { + match self { + Self::Key(key) => key == name, + Self::Index(index) => index.to_string() == name, + } + } } #[derive(Clone, Debug, PartialEq, Eq)] @@ -1542,16 +1561,16 @@ struct ConfigProjection { } impl ConfigProjection { - fn selects(&self, scope: &[String]) -> bool { + fn selects(&self, scope: &[ConfigSegment]) -> bool { // Keep selected leaves, their ancestors and complete leaf values. self.paths.iter().any(|path| { path.iter() .zip(scope) - .all(|(expected, actual)| expected == actual) + .all(|(expected, actual)| actual.is(expected)) }) } - fn needed_from_include(&self, siblings: &Map, scope: &[String]) -> Self { + fn needed_from_include(&self, siblings: &Map, scope: &[ConfigSegment]) -> Self { Self { paths: self .paths @@ -1559,7 +1578,7 @@ impl ConfigProjection { .filter(|path| { path.iter() .zip(scope) - .all(|(expected, actual)| expected == actual) + .all(|(expected, actual)| actual.is(expected)) && (path.len() <= scope.len() || !scalar_override(siblings, &path[scope.len()..])) }) @@ -1641,7 +1660,7 @@ struct IncludeProcessor { archived_config_roots: Vec, canonical_root_dir: PathBuf, include_paths: BTreeSet, - include_scopes: BTreeMap>>, + include_scopes: BTreeMap>>, projection: Option, allowed_roots: Vec, canonical_allowed_roots: Vec, @@ -1669,13 +1688,13 @@ impl IncludeProcessor { &self, base: ResolvedInclude, next: ResolvedInclude, - scope: &[String], + scope: &[ConfigSegment], ) -> ResolvedInclude { let masks_previous = self.projection.as_ref().is_some_and(|projection| { projection .paths .iter() - .filter(|path| path.iter().zip(scope).all(|(a, b)| a == b)) + .filter(|path| path.iter().zip(scope).all(|(a, b)| b.is(a))) .all(|path| { scalar_value_override(&next.value, path.get(scope.len()..).unwrap_or(&[])) }) @@ -1694,14 +1713,14 @@ impl IncludeProcessor { containing_file: &Path, depth: usize, stack: &mut Vec, - scope: &[String], + scope: &[ConfigSegment], ) -> Result { if let Some(projection) = &self.projection { if !projection.selects(scope) { return Ok(ResolvedInclude::known(Value::Null)); } for path in &projection.paths { - if path.len() >= scope.len() && path.iter().zip(scope).all(|(a, b)| a == b) { + if path.len() >= scope.len() && path.iter().zip(scope).all(|(a, b)| b.is(a)) { let mut observed = Some(&value); for key in &path[scope.len()..] { observed = observed.and_then(|v| v.get(key)); @@ -1716,7 +1735,7 @@ impl IncludeProcessor { let mut outside = None; for (index, value) in values.into_iter().enumerate() { let mut child_scope = scope.to_vec(); - child_scope.push(index.to_string()); + child_scope.push(ConfigSegment::Index(index)); let child = self.process(value, containing_file, depth, stack, &child_scope)?; outside = outside.or(child.outside); result.push(child.value); @@ -1734,7 +1753,7 @@ impl IncludeProcessor { return true; } let mut child_scope = scope.to_vec(); - child_scope.push(key.clone()); + child_scope.push(ConfigSegment::Key(key.clone())); projection.selects(&child_scope) }); } @@ -1771,7 +1790,7 @@ impl IncludeProcessor { let mut outside = None; for (key, value) in object { let mut child_scope = scope.to_vec(); - child_scope.push(key.clone()); + child_scope.push(ConfigSegment::Key(key.clone())); let child = self.process(value, containing_file, depth, stack, &child_scope)?; outside = outside.or(child.outside); siblings.insert(key, child.value); @@ -1796,7 +1815,7 @@ impl IncludeProcessor { containing_file: &Path, depth: usize, stack: &mut Vec, - scope: &[String], + scope: &[ConfigSegment], ) -> Result { match include { Value::String(path) => self.load_include(&path, containing_file, depth, stack, scope), @@ -1821,7 +1840,7 @@ impl IncludeProcessor { containing_file: &Path, depth: usize, stack: &mut Vec, - scope: &[String], + scope: &[ConfigSegment], ) -> Result { if depth >= MAX_INCLUDE_DEPTH { return Err(format!( diff --git a/tests/migrate_command_tests.rs b/tests/migrate_command_tests.rs index 8d9dff01..9ec647a0 100644 --- a/tests/migrate_command_tests.rs +++ b/tests/migrate_command_tests.rs @@ -5187,12 +5187,11 @@ fn migrate_and_adopt_keep_home_relative_paths_outside_the_state_dir_pointing_at_ copied["hooks"]["internal"]["load"]["extraDirs"], serde_json::json!([format!("{home_path}/hooks")]) ); - // These consumers expand `~` against the OS home or resolve links - // first, so the values already keep their meaning. assert_eq!( copied["nodeHost"]["workerRuns"]["folders"], - serde_json::json!(["~/work"]) + serde_json::json!([format!("{home_path}/work")]) ); + // Expanded against the OS home, which the environment keeps. assert_eq!( copied["memory"]["search"]["extraPaths"], serde_json::json!(["~/notes"]) @@ -5380,7 +5379,7 @@ fn migrate_and_adopt_resolve_parent_traversal_from_the_supplied_home_spelling() } #[test] -fn migrate_and_adopt_keep_home_relative_paths_in_an_include_at_a_list_entry_as_written() { +fn migrate_and_adopt_expand_home_relative_paths_in_includes_at_list_entries() { for adopt in [false, true] { let root = TestDir::new("migrate-home-relative-list-include"); let env = ocm_env(&root); @@ -5388,11 +5387,72 @@ fn migrate_and_adopt_keep_home_relative_paths_in_an_include_at_a_list_entry_as_w let source = home.join(".openclaw"); fs::create_dir_all(&source).unwrap(); let config = serde_json::json!({ - "skills": {"load": {"extraDirs": [{"$include": "./skills-dir.json"}]}} + "skills": {"load": {"extraDirs": [{"$include": "./skills-dir.json"}]}}, + // The two includes concatenate `items`: the first gives `paths` a + // digit-only key, the second a list entry from its own include. + "hooks": {"$include": ["./a.json", "./b.json"]} + }); + support::write_text(&source.join("openclaw.json"), &config.to_string()); + let files = [ + ("skills-dir.json", r#""~/shared-skills""#), + ("a.json", r#"{"items":[{"paths":{"0":"~/literal"}}]}"#), + ( + "b.json", + r#"{"items":[{"paths":[{"$include":"./p.json"}]}]}"#, + ), + ("p.json", r#""~/bin""#), + ]; + for (name, contents) in files { + support::write_text(&source.join(name), contents); + } + + let imported = run_ocm( + root.path(), + &env, + &migrate_or_adopt_args(adopt, "copied", &source), + ); + assert!(imported.status.success(), "{}", stderr(&imported)); + + let target = root.child("ocm-home/envs/copied/.openclaw"); + let read = |name: &str| { + serde_json::from_str::( + &fs::read_to_string(target.join(name)).unwrap(), + ) + .unwrap() + }; + let home_path = home.to_str().unwrap(); + assert_eq!( + read("skills-dir.json"), + format!("{home_path}/shared-skills"), + "adopt={adopt}" + ); + assert_eq!(read("p.json"), format!("{home_path}/bin"), "adopt={adopt}"); + // A digit-only object key is not a `paths` list entry. + assert_eq!(read("a.json")["items"][0]["paths"]["0"], "~/literal"); + for (name, contents) in files { + assert_eq!(fs::read_to_string(source.join(name)).unwrap(), contents); + } + } +} + +#[test] +fn migrate_and_adopt_expand_home_relative_paths_with_placeholders_before_substitution() { + for adopt in [false, true] { + let root = TestDir::new("migrate-home-relative-placeholders"); + let env = ocm_env(&root); + let home = root.child("home"); + let source = home.join(".openclaw"); + fs::create_dir_all(&source).unwrap(); + let config = serde_json::json!({ + "skills": {"load": {"extraDirs": [ + "~/${SKILLS_SUB}/shared", + "~/up/../${UP}/../shared", + "~/.openclaw/${SKILLS_SUB}", + ]}}, + "logging": {"file": "~/logs/${LOG_NAME}.log"}, + "messages": {"responsePrefix": "~/${NAME}"} }); support::write_text(&source.join("openclaw.json"), &config.to_string()); - let include = "\"~/shared-skills\""; - support::write_text(&source.join("skills-dir.json"), include); let imported = run_ocm( root.path(), @@ -5403,14 +5463,167 @@ fn migrate_and_adopt_keep_home_relative_paths_in_an_include_at_a_list_entry_as_w let target = root.child("ocm-home/envs/copied/.openclaw"); let copied: serde_json::Value = - serde_json::from_str(&fs::read_to_string(target.join("skills-dir.json")).unwrap()) + serde_json::from_str(&fs::read_to_string(target.join("openclaw.json")).unwrap()) .unwrap(); - // A recorded include index cannot be told apart from a digit-only - // key, so the value keeps its spelling (openclaw/ocm#972). - assert_eq!(copied, "~/shared-skills", "adopt={adopt}"); + let home_path = home.to_str().unwrap(); + // Only the literal directories before a placeholder are resolved; + // OpenClaw resolves the rest after substituting it. assert_eq!( - fs::read_to_string(source.join("skills-dir.json")).unwrap(), - include + copied["skills"]["load"]["extraDirs"], + serde_json::json!([ + format!("{home_path}/${{SKILLS_SUB}}/shared"), + format!("{home_path}/${{UP}}/../shared"), + "~/.openclaw/${SKILLS_SUB}", + ]), + "adopt={adopt}" + ); + assert_eq!( + copied["logging"]["file"], + format!("{home_path}/logs/${{LOG_NAME}}.log") + ); + assert_eq!(copied["messages"]["responsePrefix"], "~/${NAME}"); + } +} + +#[cfg(unix)] +#[test] +fn migrate_and_adopt_expand_worker_folders_without_resolving_parent_traversal() { + for adopt in [false, true] { + let root = TestDir::new("migrate-home-relative-worker-folders"); + let env = ocm_env(&root); + let home = root.child("home"); + let source = home.join(".openclaw"); + fs::create_dir_all(source.join("wf")).unwrap(); + fs::create_dir_all(home.join("data/deep")).unwrap(); + fs::create_dir_all(home.join("data/work")).unwrap(); + fs::create_dir_all(home.join("work")).unwrap(); + // OpenClaw resolves `hop` before `..`, so `~/hop/../work` names + // `~/data/work`, not `~/work`. + std::os::unix::fs::symlink(home.join("data/deep"), home.join("hop")).unwrap(); + let config = serde_json::json!({ + "nodeHost": {"workerRuns": {"enabled": true, "folders": [ + "~/work", + "~/hop/../work", + "~/.openclaw/wf", + " ~/work", + ]}} + }); + support::write_text(&source.join("openclaw.json"), &config.to_string()); + + let imported = run_ocm( + root.path(), + &env, + &migrate_or_adopt_args(adopt, "copied", &source), + ); + assert!(imported.status.success(), "{}", stderr(&imported)); + + let target = root.child("ocm-home/envs/copied/.openclaw"); + let copied: serde_json::Value = + serde_json::from_str(&fs::read_to_string(target.join("openclaw.json")).unwrap()) + .unwrap(); + let home_path = home.to_str().unwrap(); + let folders = &copied["nodeHost"]["workerRuns"]["folders"]; + assert_eq!( + folders, + &serde_json::json!([ + format!("{home_path}/work"), + format!("{home_path}/hop/../work"), + // Already names the environment's copy. + "~/.openclaw/wf", + // OpenClaw does not trim, so this is not home-relative. + " ~/work", + ]), + "adopt={adopt}" + ); + assert_eq!( + fs::canonicalize(folders[1].as_str().unwrap()).unwrap(), + fs::canonicalize(home.join("data/work")).unwrap() + ); + } +} + +#[test] +fn migrate_and_adopt_keep_home_relative_trust_roots_and_name_them() { + for adopt in [false, true] { + let root = TestDir::new("migrate-home-relative-trust-roots"); + let env = ocm_env(&root); + let home = root.child("home"); + let source = home.join(".openclaw"); + fs::create_dir_all(&source).unwrap(); + let home_path = home.to_str().unwrap().to_string(); + let config = serde_json::json!({ + "skills": {"load": {"allowSymlinkTargets": ["~/trusted", "~/.openclaw/skills"]}}, + "security": {"installPolicy": {"exec": {"trustedDirs": [ + {"$include": "./trusted-dir.json"}, + format!("{home_path}/bin"), + ]}}} + }); + support::write_text(&source.join("openclaw.json"), &config.to_string()); + support::write_text(&source.join("trusted-dir.json"), r#"" ~/bin""#); + + let imported = run_ocm( + root.path(), + &env, + &migrate_or_adopt_args(adopt, "copied", &source), + ); + assert!(imported.status.success(), "{}", stderr(&imported)); + + let target = root.child("ocm-home/envs/copied/.openclaw"); + let copied: serde_json::Value = + serde_json::from_str(&fs::read_to_string(target.join("openclaw.json")).unwrap()) + .unwrap(); + assert_eq!( + copied["skills"]["load"]["allowSymlinkTargets"], + serde_json::json!(["~/trusted", "~/.openclaw/skills"]), + "adopt={adopt}" + ); + assert_eq!( + fs::read_to_string(target.join("trusted-dir.json")).unwrap(), + r#"" ~/bin""# + ); + let warnings = stderr(&imported); + for setting in [ + "skills.load.allowSymlinkTargets", + "security.installPolicy.exec.trustedDirs", + ] { + assert_eq!( + warnings + .lines() + .filter(|line| line + .starts_with("warning: env copied keeps the ~/ entries of trust root") + && line.contains(&format!(" {setting} "))) + .count(), + 1, + "adopt={adopt}: {warnings}" + ); + } + } +} + +#[test] +fn migrate_and_adopt_do_not_name_trust_roots_that_keep_their_meaning() { + for adopt in [false, true] { + let root = TestDir::new("migrate-home-relative-trust-roots-unchanged"); + let env = ocm_env(&root); + let home = root.child("home"); + let source = home.join(".openclaw"); + fs::create_dir_all(&source).unwrap(); + let config = serde_json::json!({ + "skills": {"load": {"allowSymlinkTargets": ["~/.openclaw/skills", "/opt/skills"]}}, + "security": {"installPolicy": {"exec": {"trustedDirs": ["/usr/local/bin"]}}} + }); + support::write_text(&source.join("openclaw.json"), &config.to_string()); + + let imported = run_ocm( + root.path(), + &env, + &migrate_or_adopt_args(adopt, "copied", &source), + ); + assert!(imported.status.success(), "{}", stderr(&imported)); + assert!( + !stderr(&imported).contains("trust root"), + "adopt={adopt}: {}", + stderr(&imported) ); } } From 562baddd49a73ec971e7852f89e251539f34f285 Mon Sep 17 00:00:00 2001 From: Shakker Date: Mon, 2 Nov 2026 11:34:21 +0000 Subject: [PATCH 2/8] test: worker folder leaving the state dir through .. keeps pointing outside it after migrate --- tests/migrate_command_tests.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/migrate_command_tests.rs b/tests/migrate_command_tests.rs index 9ec647a0..750120f5 100644 --- a/tests/migrate_command_tests.rs +++ b/tests/migrate_command_tests.rs @@ -5505,6 +5505,7 @@ fn migrate_and_adopt_expand_worker_folders_without_resolving_parent_traversal() "~/work", "~/hop/../work", "~/.openclaw/wf", + "~/.openclaw/../work", " ~/work", ]}} }); @@ -5530,6 +5531,9 @@ fn migrate_and_adopt_expand_worker_folders_without_resolving_parent_traversal() format!("{home_path}/hop/../work"), // Already names the environment's copy. "~/.openclaw/wf", + // Leaves the state directory, so the relocation of copied + // state paths keeps it. + format!("{home_path}/.openclaw/../work"), // OpenClaw does not trim, so this is not home-relative. " ~/work", ]), From c20f4c46da1ac72560bd15b70e27792aad283b79 Mon Sep 17 00:00:00 2001 From: Shakker Date: Mon, 2 Nov 2026 11:45:41 +0000 Subject: [PATCH 3/8] fix: name kept ~/.openclaw placeholder paths, keep rejected worker folders, handle root paths A `~/.openclaw/...` value with a `${VAR}` placeholder stays as written and is now named in a migration warning, because the placeholder may lead outside the state directory. A worker folder under `~/.openclaw` whose `..` does not lead to an existing directory stays as written, so OpenClaw keeps rejecting it instead of the copied target. A value resolving to the filesystem root no longer panics. --- docs/USAGE.md | 31 +++--- src/cli/migrate.rs | 28 +++-- src/migrate.rs | 10 +- src/store/mod.rs | 18 ++-- src/store/openclaw_config.rs | 189 +++++++++++++++++++++++---------- tests/migrate_command_tests.rs | 25 ++++- 6 files changed, 205 insertions(+), 96 deletions(-) diff --git a/docs/USAGE.md b/docs/USAGE.md index 306dc96c..8cda041e 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -1289,23 +1289,28 @@ handling and lexical `.` and `..` resolution as its OpenClaw consumer. Settings OpenClaw expands against the OS home keep their meaning without a rewrite, because the environment keeps your `HOME`. Other strings, such as message text, stay as written. Path values in an include placed at a list position are -rewritten like any other. In a value containing a `${VAR}` placeholder, only the directories before the -first placeholder are resolved; OpenClaw substitutes the placeholder and -resolves the rest, as it would for the same value spelled from your home, so a -placeholder that itself leads into `~/.openclaw` names the source's state -directory. `nodeHost.workerRuns.folders` entries, which OpenClaw resolves -through symlinks, only have their leading `~` replaced, so a link before `..` -still decides where they lead. The trust roots `skills.load.allowSymlinkTargets` -and `security.installPolicy.exec.trustedDirs` are not rewritten, because that -would change what the new environment trusts: their `~/...` entries outside +rewritten like any other. In a value containing a `${VAR}` placeholder, only +the directories before the first placeholder are resolved; OpenClaw +substitutes the placeholder and resolves the rest, as it would for the same +value spelled from your home, so a placeholder that itself leads into +`~/.openclaw` names the source's state directory. A placeholder value under +`~/.openclaw` stays as written, and migration prints a warning naming its +setting, because its placeholder may still lead outside `~/.openclaw`. +`nodeHost.workerRuns.folders` entries, which OpenClaw resolves through +symlinks, only have their leading `~` replaced, so a link before `..` still +decides where they lead; an entry under `~/.openclaw` whose `..` does not lead +to an existing directory stays as written, so OpenClaw keeps rejecting it. The +trust roots `skills.load.allowSymlinkTargets` and +`security.installPolicy.exec.trustedDirs` are not rewritten, because that would +change what the new environment trusts: their `~/...` entries outside `~/.openclaw` now resolve under the environment root, and migration prints a warning naming each such setting. Replace those entries with absolute paths to keep trusting the original directories. The source home is your home directory when the migrated directory is your own `~/.openclaw`, through any -path that leads to it; -otherwise it is the directory that contains the migrated `.openclaw` as you -spelled it, or as it is stored when only that path is named `.openclaw`, or -your home directory for a state directory with another name. Values under +path that leads to it; otherwise it is the directory that contains the +migrated `.openclaw` as you spelled it, or as it is stored when only that path +is named `.openclaw`, or your home directory for a state directory with another +name. Values under `~/.openclaw` already name the environment's copied state and stay as written. Clone and archive import keep `~/...` values unchanged, because their source already expands `~` against an environment root. diff --git a/src/cli/migrate.rs b/src/cli/migrate.rs index a5a14f97..66eee1bc 100644 --- a/src/cli/migrate.rs +++ b/src/cli/migrate.rs @@ -169,9 +169,9 @@ impl Cli { result.sandbox_port, ); self.warn_external_migration_plugins(&result.external_plugin_ids); - self.warn_kept_home_relative_trust_roots( + self.warn_kept_home_relative_settings( &result.summary.name, - &result.kept_home_relative_trust_roots, + &result.kept_home_relative_settings, ); let summary = result.summary; @@ -215,9 +215,9 @@ impl Cli { result.sandbox_port, ); self.warn_external_migration_plugins(&result.external_plugin_ids); - self.warn_kept_home_relative_trust_roots( + self.warn_kept_home_relative_settings( &result.summary.name, - &result.kept_home_relative_trust_roots, + &result.kept_home_relative_settings, ); let summary = result.summary; @@ -251,11 +251,21 @@ impl Cli { } } - fn warn_kept_home_relative_trust_roots(&self, env_name: &str, settings: &[String]) { - for setting in settings { - self.stderr_line(format!( - "warning: env {env_name} keeps the ~/ entries of trust root {setting} as written; they now resolve under the env root instead of the source home, so OCM did not carry that trust over. Replace them with absolute paths to keep trusting the original directories" - )); + fn warn_kept_home_relative_settings( + &self, + env_name: &str, + settings: &[crate::store::KeptHomeRelativeSetting], + ) { + for kept in settings { + let setting = &kept.setting; + self.stderr_line(match kept.reason { + crate::store::KeptHomeRelativeReason::TrustRoot => format!( + "warning: env {env_name} keeps the ~/ entries of trust root {setting} as written; they now resolve under the env root instead of the source home, so OCM did not carry that trust over. Replace them with absolute paths to keep trusting the original directories" + ), + crate::store::KeptHomeRelativeReason::Placeholder => format!( + "warning: env {env_name} keeps the ~/.openclaw entries of {setting} that contain a ${{VAR}} placeholder as written; if a placeholder leads outside ~/.openclaw, the entry now resolves under the env root instead of the source home. Replace such entries with absolute paths" + ), + }); } } diff --git a/src/migrate.rs b/src/migrate.rs index d3e44dcb..f78e4d45 100644 --- a/src/migrate.rs +++ b/src/migrate.rs @@ -50,7 +50,7 @@ pub(crate) struct MigrateHomeResult { pub cleared_sandbox_origin: bool, pub sandbox_port: Option, pub external_plugin_ids: Vec, - pub kept_home_relative_trust_roots: Vec, + pub kept_home_relative_settings: Vec, } #[derive(Clone, Debug)] @@ -354,7 +354,7 @@ struct CompletedMigrationImport { cleared_sandbox_origin: bool, sandbox_port: Option, external_plugin_ids: Vec, - kept_home_relative_trust_roots: Vec, + kept_home_relative_settings: Vec, } pub fn default_migration_source_home(env: &BTreeMap) -> PathBuf { @@ -520,7 +520,7 @@ fn migrate_plain_openclaw_home_inner( cleared_sandbox_origin: completed.cleared_sandbox_origin, sandbox_port: completed.sandbox_port, external_plugin_ids: completed.external_plugin_ids, - kept_home_relative_trust_roots: completed.kept_home_relative_trust_roots, + kept_home_relative_settings: completed.kept_home_relative_settings, }) } @@ -649,7 +649,7 @@ fn complete_migration_import( cleared_sandbox_origin: config_rewrite.cleared_sandbox_origin, sandbox_port: config_rewrite.sandbox_port, external_plugin_ids: runtime_state.external_plugin_ids, - kept_home_relative_trust_roots: config_rewrite.kept_home_relative_trust_roots, + kept_home_relative_settings: config_rewrite.kept_home_relative_settings, }); }; @@ -675,7 +675,7 @@ fn complete_migration_import( cleared_sandbox_origin: config_rewrite.cleared_sandbox_origin, sandbox_port: config_rewrite.sandbox_port, external_plugin_ids: runtime_state.external_plugin_ids, - kept_home_relative_trust_roots: config_rewrite.kept_home_relative_trust_roots, + kept_home_relative_settings: config_rewrite.kept_home_relative_settings, }), Err(error) => { let rollback = rollback_migrated_launcher(created_launcher.as_deref(), env, cwd); diff --git a/src/store/mod.rs b/src/store/mod.rs index 6bcdd511..a9def9fe 100644 --- a/src/store/mod.rs +++ b/src/store/mod.rs @@ -84,15 +84,15 @@ pub(crate) use layout::{ supervisor_child_log_file, }; pub(crate) use openclaw_config::{ - GatewayReadinessHost, MinimumLocalConfig, OpenClawConfigAudit, OpenClawConfigRepair, - audit_openclaw_config, clear_skip_bootstrap_for_openclaw_onboarding, dev_ui_gateway_base_path, - dev_ui_gateway_url, ensure_minimum_local_openclaw_config, gateway_readiness_certificate, - gateway_readiness_host, gateway_url_for_launch, loopback_gateway_url, - normalize_new_environment_sandbox_origin, openclaw_config_include_paths, - openclaw_config_uses_includes, prepare_minimum_local_openclaw_config, - prepare_openclaw_config_repair, reject_include_owned_agent_workspaces, - reject_include_owned_sandbox_origin, rewrite_copied_config_include_directives, - rewrite_external_workspace_paths_for_migration, + GatewayReadinessHost, KeptHomeRelativeReason, KeptHomeRelativeSetting, MinimumLocalConfig, + OpenClawConfigAudit, OpenClawConfigRepair, audit_openclaw_config, + clear_skip_bootstrap_for_openclaw_onboarding, dev_ui_gateway_base_path, dev_ui_gateway_url, + ensure_minimum_local_openclaw_config, gateway_readiness_certificate, gateway_readiness_host, + gateway_url_for_launch, loopback_gateway_url, normalize_new_environment_sandbox_origin, + openclaw_config_include_paths, openclaw_config_uses_includes, + prepare_minimum_local_openclaw_config, prepare_openclaw_config_repair, + reject_include_owned_agent_workspaces, reject_include_owned_sandbox_origin, + rewrite_copied_config_include_directives, rewrite_external_workspace_paths_for_migration, rewrite_identity_bound_workspace_paths_for_target, rewrite_openclaw_config_for_migration, rewrite_openclaw_config_for_new_environment, rewrite_openclaw_config_for_simulation, rewrite_openclaw_config_for_target, rewrite_openclaw_config_includes_for_target, diff --git a/src/store/openclaw_config.rs b/src/store/openclaw_config.rs index 5f3b9894..14c86dcb 100644 --- a/src/store/openclaw_config.rs +++ b/src/store/openclaw_config.rs @@ -38,9 +38,27 @@ pub(crate) struct OpenClawConfigAudit { pub(crate) struct OpenClawConfigRewriteOutcome { pub cleared_sandbox_origin: bool, pub sandbox_port: Option, - /// Trust-root settings whose home-relative entries migration kept as - /// written, so they now resolve under the environment root. - pub kept_home_relative_trust_roots: Vec, + /// Settings whose home-relative entries migration kept as written + /// although they may now resolve under the environment root. + pub kept_home_relative_settings: Vec, +} + +/// A setting whose home-relative entries migration kept as written, because +/// spelling them out could change what it means. +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub(crate) struct KeptHomeRelativeSetting { + /// The setting's path, with `[]` for a list entry. + pub setting: String, + pub reason: KeptHomeRelativeReason, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)] +pub(crate) enum KeptHomeRelativeReason { + /// A trust root: spelling it out would change what the environment trusts. + TrustRoot, + /// A value under `~/.openclaw` with a `${VAR}` placeholder, which may + /// lead outside the state directory once substituted. + Placeholder, } pub(crate) fn audit_openclaw_config( @@ -214,7 +232,7 @@ pub(crate) fn rewrite_openclaw_config_for_migration( gateway_port: Option, sandbox_origin: Option<&str>, ) -> Result { - let kept_trust_roots = expand_migrated_home_relative_paths(target_paths, source_home)?; + let kept = expand_migrated_home_relative_paths(target_paths, source_home)?; rewrite_migrated_plugin_load_paths( target_paths, source_state_root, @@ -226,7 +244,7 @@ pub(crate) fn rewrite_openclaw_config_for_migration( gateway_port, SandboxOriginPolicy::NewEnvironment(sandbox_origin), )?; - outcome.kept_home_relative_trust_roots = kept_trust_roots.into_iter().collect(); + outcome.kept_home_relative_settings = kept.into_iter().collect(); Ok(outcome) } @@ -236,18 +254,18 @@ pub(crate) fn rewrite_openclaw_config_for_migration( /// directory; every other home-relative path setting is spelled out against /// the source home so it keeps naming the same location. Absolute values /// inside the source state directory are then relocated by the later rewrites. -/// Returns the trust-root settings whose home-relative entries were kept as -/// written. +/// Returns the settings whose home-relative entries were kept as written +/// although they may now resolve elsewhere. fn expand_migrated_home_relative_paths( target: &EnvPaths, source_home: &Path, -) -> Result, String> { - let mut kept_trust_roots = BTreeSet::new(); +) -> Result, String> { + let mut kept = BTreeSet::new(); if !source_home.is_absolute() { - return Ok(kept_trust_roots); + return Ok(kept); } let Some(effective) = load_effective_openclaw_config(&target.config_path)? else { - return Ok(kept_trust_roots); + return Ok(kept); }; let mut scopes = effective.include_scopes; scopes.insert(target.config_path.clone(), BTreeSet::from([Vec::new()])); @@ -258,12 +276,7 @@ fn expand_migrated_home_relative_paths( let mut rewritten = None; for mut scope in scopes { let mut candidate = value.clone(); - expand_home_relative_paths_in_scope( - &mut candidate, - &mut scope, - source_home, - &mut kept_trust_roots, - ); + expand_home_relative_paths_in_scope(&mut candidate, &mut scope, source_home, &mut kept); if rewritten .as_ref() .is_some_and(|previous| previous != &candidate) @@ -279,34 +292,48 @@ fn expand_migrated_home_relative_paths( write_copied_config_value(&path, &target.state_dir, &rewritten)?; } } - Ok(kept_trust_roots) + Ok(kept) } /// Expands the home-relative path settings in a config fragment included at -/// `scope`, and records the trust-root settings it keeps as written. -/// `$include` directives keep their own relocation. +/// `scope`, and records the settings it keeps as written although they may +/// now resolve elsewhere. `$include` directives keep their own relocation. fn expand_home_relative_paths_in_scope( value: &mut Value, scope: &mut Vec, home: &Path, - kept_trust_roots: &mut BTreeSet, + kept: &mut BTreeSet, ) -> bool { match value { Value::String(raw) => { - let expanded = match home_relative_path_setting(scope) { - Some(HomePathRule::Lexical { trims }) => { - expand_home_relative_path(raw, home, trims) + let Some(rule) = home_relative_path_setting(scope) else { + return false; + }; + let (expanded, trims) = match rule { + HomePathRule::Lexical { trims } => { + (expand_home_relative_path(raw, home, trims), trims) } - Some(HomePathRule::Physical) => expand_physical_home_relative_path(raw, home), - Some(HomePathRule::TrustRoot(setting)) => { - if expand_home_relative_path(raw, home, true).is_some() { - kept_trust_roots.insert(setting.to_string()); + HomePathRule::Physical => (expand_physical_home_relative_path(raw, home), false), + HomePathRule::TrustRoot => { + let moves = expand_home_relative_path(raw, home, true).is_some() + || has_home_relative_placeholder(raw, true); + if moves { + kept.insert(KeptHomeRelativeSetting { + setting: config_setting_name(scope), + reason: KeptHomeRelativeReason::TrustRoot, + }); } - None + return false; } - None => None, }; let Some(expanded) = expanded else { + // A placeholder under `~/.openclaw` can still lead outside it. + if has_home_relative_placeholder(raw, trims) { + kept.insert(KeptHomeRelativeSetting { + setting: config_setting_name(scope), + reason: KeptHomeRelativeReason::Placeholder, + }); + } return false; }; *raw = expanded; @@ -316,8 +343,7 @@ fn expand_home_relative_paths_in_scope( let mut changed = false; for (index, nested) in values.iter_mut().enumerate() { scope.push(ConfigSegment::Index(index)); - changed |= - expand_home_relative_paths_in_scope(nested, scope, home, kept_trust_roots); + changed |= expand_home_relative_paths_in_scope(nested, scope, home, kept); scope.pop(); } changed @@ -326,8 +352,7 @@ fn expand_home_relative_paths_in_scope( let mut changed = false; for (key, nested) in values.iter_mut().filter(|(key, _)| *key != "$include") { scope.push(ConfigSegment::Key(key.clone())); - changed |= - expand_home_relative_paths_in_scope(nested, scope, home, kept_trust_roots); + changed |= expand_home_relative_paths_in_scope(nested, scope, home, kept); scope.pop(); } changed @@ -345,9 +370,26 @@ enum HomePathRule { /// The leading `~` is replaced and the path is resolved through links /// (`realpath`), so a link before `..` decides where it leads. Physical, - /// A trust root, named here. Spelling it out would change what the - /// environment trusts, so it stays as written and is reported. - TrustRoot(&'static str), + /// A trust root. Spelling it out would change what the environment + /// trusts, so it stays as written and is reported. + TrustRoot, +} + +/// The setting at `scope` as users write it, with `[]` for a list entry. +fn config_setting_name(scope: &[ConfigSegment]) -> String { + let mut name = String::new(); + for segment in scope { + match segment { + ConfigSegment::Key(key) => { + if !name.is_empty() { + name.push('.'); + } + name.push_str(key); + } + ConfigSegment::Index(_) => name.push_str("[]"), + } + } + name } /// Settings whose own OpenClaw consumers expand `~` against `OPENCLAW_HOME` @@ -398,11 +440,11 @@ const CONSUMER_HOME_PATH_SETTINGS: &[(&[&str], HomePathRule)] = &[ // src/skills/loading/symlink-targets.ts and src/security/install-policy.ts ( &["skills", "load", "allowSymlinkTargets", "[]"], - HomePathRule::TrustRoot("skills.load.allowSymlinkTargets"), + HomePathRule::TrustRoot, ), ( &["security", "installPolicy", "exec", "trustedDirs", "[]"], - HomePathRule::TrustRoot("security.installPolicy.exec.trustedDirs"), + HomePathRule::TrustRoot, ), ]; @@ -473,6 +515,17 @@ fn is_home_relative(value: &str) -> bool { value == "~" || value.starts_with("~/") || value.starts_with("~\\") } +/// Whether OpenClaw would read `raw` as a home-relative value containing a +/// `${VAR}` placeholder. +fn has_home_relative_placeholder(raw: &str, trims: bool) -> bool { + let value = if trims { + raw.trim_matches(is_ecmascript_whitespace) + } else { + raw + }; + is_home_relative(value) && value.contains("${") +} + /// Splits a home-relative value before the separator that precedes its /// first `${VAR}` placeholder. OpenClaw substitutes placeholders before it /// resolves the path, so a placeholder's value can still add `..` or name @@ -512,32 +565,37 @@ fn expand_home_relative_path(raw: &str, home: &Path, trims: bool) -> Option Option { if !is_home_relative(raw) { return None; } - let (literal, _) = split_before_placeholder(raw); + let (literal, rest) = split_before_placeholder(raw); let mut literal_path = home.as_os_str().to_os_string(); literal_path.push(&literal[1..]); let literal_path = Path::new(&literal_path); + let expanded = format!("{}{}", home.to_str()?, &raw[1..]); if literal_path.starts_with(home.join(".openclaw")) - && !literal_path - .components() - .any(|component| component == std::path::Component::ParentDir) + && (!rest.is_empty() + || !literal_path + .components() + .any(|component| component == std::path::Component::ParentDir) + || fs::canonicalize(&expanded).is_err()) { return None; } - Some(format!("{}{}", home.to_str()?, &raw[1..])) + Some(expanded) } /// Moves a Gateway port that a copied `$include` supplies to `gateway_port`, @@ -3094,6 +3152,17 @@ mod tests { Some("/home/alice/dev/p${N}.log") ); assert_eq!(expand("~/../../${X}").as_deref(), Some("/${X}")); + // A value that resolves to the filesystem root has nothing after it. + assert_eq!(expand("~/../..").as_deref(), Some("/")); + assert_eq!( + expand_home_relative_path("~", Path::new("/"), true).as_deref(), + Some("/") + ); + // On POSIX `\` is part of the name, which stays after a root home. + assert_eq!( + expand_home_relative_path("~\\${X}", Path::new("/"), true).as_deref(), + Some("/\\${X}") + ); assert_eq!(expand("~/.openclaw/${SUB}/x"), None); assert_eq!( expand("~/.openclaw/../${SUB}").as_deref(), @@ -3152,13 +3221,11 @@ mod tests { ); assert_eq!( setting(&["skills", "load", "allowSymlinkTargets", "[]"]), - Some(HomePathRule::TrustRoot("skills.load.allowSymlinkTargets")) + Some(HomePathRule::TrustRoot) ); assert_eq!( setting(&["security", "installPolicy", "exec", "trustedDirs", "[]"]), - Some(HomePathRule::TrustRoot( - "security.installPolicy.exec.trustedDirs" - )) + Some(HomePathRule::TrustRoot) ); for text in [ &["messages", "responsePrefix"][..], @@ -3201,17 +3268,21 @@ mod tests { expand("~/hop/../work").as_deref(), Some("/home/alice/hop/../work") ); - assert_eq!( - expand("~/.openclaw/../work").as_deref(), - Some("/home/alice/.openclaw/../work") - ); + // Under `~/.openclaw`, `..` is spelled out only when it leads to an + // existing directory; this home does not exist. + assert_eq!(expand("~/.openclaw/../work"), None); assert_eq!( expand("~/${SUB}/work").as_deref(), Some("/home/alice/${SUB}/work") ); // The consumer does not trim. assert_eq!(expand(" ~/work"), None); - for copied in ["~/.openclaw", "~/.openclaw/wf", "~/.openclaw/${SUB}"] { + for copied in [ + "~/.openclaw", + "~/.openclaw/wf", + "~/.openclaw/${SUB}", + "~/.openclaw/a/../${SUB}", + ] { assert_eq!(expand(copied), None, "{copied}"); } assert_eq!( @@ -3259,6 +3330,10 @@ mod tests { home_relative_path_setting(scopes.first().unwrap()), Some(HomePathRule::Lexical { trims: true }) ); + assert_eq!( + config_setting_name(scopes.first().unwrap()), + "hooks.items[].paths[]" + ); } fn meta(name: &str, root: &str, gateway_port: Option) -> EnvMeta { diff --git a/tests/migrate_command_tests.rs b/tests/migrate_command_tests.rs index 750120f5..7b96a438 100644 --- a/tests/migrate_command_tests.rs +++ b/tests/migrate_command_tests.rs @@ -5482,6 +5482,20 @@ fn migrate_and_adopt_expand_home_relative_paths_with_placeholders_before_substit format!("{home_path}/logs/${{LOG_NAME}}.log") ); assert_eq!(copied["messages"]["responsePrefix"], "~/${NAME}"); + // The kept entry is named, because its placeholder may leave + // `~/.openclaw` once substituted. + let warnings = stderr(&imported); + assert_eq!( + warnings + .lines() + .filter(|line| line.starts_with( + "warning: env copied keeps the ~/.openclaw entries of skills.load.extraDirs[] that contain a ${VAR} placeholder" + )) + .count(), + 1, + "adopt={adopt}: {warnings}" + ); + assert!(!warnings.contains("logging.file"), "{warnings}"); } } @@ -5506,6 +5520,8 @@ fn migrate_and_adopt_expand_worker_folders_without_resolving_parent_traversal() "~/hop/../work", "~/.openclaw/wf", "~/.openclaw/../work", + // `missing` does not exist, so OpenClaw rejects this entry. + "~/.openclaw/missing/../wf", " ~/work", ]}} }); @@ -5534,6 +5550,9 @@ fn migrate_and_adopt_expand_worker_folders_without_resolving_parent_traversal() // Leaves the state directory, so the relocation of copied // state paths keeps it. format!("{home_path}/.openclaw/../work"), + // Kept, so it still names a missing directory in the copy + // instead of being resolved to the copied `wf`. + "~/.openclaw/missing/../wf", // OpenClaw does not trim, so this is not home-relative. " ~/work", ]), @@ -5593,9 +5612,9 @@ fn migrate_and_adopt_keep_home_relative_trust_roots_and_name_them() { assert_eq!( warnings .lines() - .filter(|line| line - .starts_with("warning: env copied keeps the ~/ entries of trust root") - && line.contains(&format!(" {setting} "))) + .filter(|line| line.starts_with(&format!( + "warning: env copied keeps the ~/ entries of trust root {setting}[] as written" + ))) .count(), 1, "adopt={adopt}: {warnings}" From f93feb590423ce7b7ba05dad49a600b6e34a4031 Mon Sep 17 00:00:00 2001 From: Shakker Date: Mon, 2 Nov 2026 11:54:34 +0000 Subject: [PATCH 4/8] fix: keep only worker folders the state relocation would move over a missing directory A worker folder whose `..` leaves `~/.openclaw` is spelled out again, so it keeps naming the plain install's target instead of a directory under the env root. Only an entry the later relocation would move into the copied state although its source path does not resolve stays as written. Placeholder entries in trust roots are covered by tests, and a list-entry include shared by a path list and a trust root is pinned as refused. --- CHANGELOG.md | 2 +- docs/USAGE.md | 30 +++++----- src/store/openclaw_config.rs | 85 ++++++++++++++++++++-------- src/store/openclaw_workspaces.rs | 7 ++- tests/migrate_command_tests.rs | 95 ++++++++++++++++++++++++++++++++ 5 files changed, 181 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d58b177d..664d3f75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ All notable changes to OCM are documented here. - Find the installed npm launcher by default when building and installing runtimes on Windows. - Remove the hidden staging file and SQLite snapshot copies when `ocm env export` is stopped by SIGINT, SIGTERM or SIGHUP, and exit with status 130. Signals ignored at startup, as under `nohup`, stay ignored. - Keep `~/...` config paths outside `~/.openclaw`, such as local plugin, extra skills, agent directory and session or cron store paths, pointing at the same location after `migrate` and `adopt import`, instead of letting them resolve under the new environment root. Other config text is left unchanged. -- Also keep `~/...` values in `nodeHost.workerRuns.folders`, in includes placed at a list position and in values containing a `${VAR}` placeholder pointing at the same location after `migrate` and `adopt import`. Home-relative trust roots in `skills.load.allowSymlinkTargets` and `security.installPolicy.exec.trustedDirs` stay as written, and migration now warns that they resolve under the new environment root. +- Also keep `~/...` values in `nodeHost.workerRuns.folders`, in includes placed at a list position and in values containing a `${VAR}` placeholder outside `~/.openclaw` pointing at the same location after `migrate` and `adopt import`. Home-relative trust roots in `skills.load.allowSymlinkTargets` and `security.installPolicy.exec.trustedDirs`, and `~/.openclaw/...` values containing a placeholder, stay as written, and migration now prints a warning naming each such setting, because they may resolve under the new environment root. ## 0.3.8 - 2026-10-29 diff --git a/docs/USAGE.md b/docs/USAGE.md index 8cda041e..aa6e3b7b 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -1298,20 +1298,22 @@ value spelled from your home, so a placeholder that itself leads into setting, because its placeholder may still lead outside `~/.openclaw`. `nodeHost.workerRuns.folders` entries, which OpenClaw resolves through symlinks, only have their leading `~` replaced, so a link before `..` still -decides where they lead; an entry under `~/.openclaw` whose `..` does not lead -to an existing directory stays as written, so OpenClaw keeps rejecting it. The -trust roots `skills.load.allowSymlinkTargets` and -`security.installPolicy.exec.trustedDirs` are not rewritten, because that would -change what the new environment trusts: their `~/...` entries outside -`~/.openclaw` now resolve under the environment root, and migration prints a -warning naming each such setting. Replace those entries with absolute paths to -keep trusting the original directories. The source home is your home -directory when the migrated directory is your own `~/.openclaw`, through any -path that leads to it; otherwise it is the directory that contains the -migrated `.openclaw` as you spelled it, or as it is stored when only that path -is named `.openclaw`, or your home directory for a state directory with another -name. Values under -`~/.openclaw` already name the environment's copied state and stay as written. +decides where they lead; an entry that then names a directory inside +`~/.openclaw` moves to the environment's copy like any other state path, and +one that would only reach the copy by stepping over a directory missing from +`~/.openclaw` stays as written, so OpenClaw keeps rejecting it. The trust roots +`skills.load.allowSymlinkTargets` and `security.installPolicy.exec.trustedDirs` +are not rewritten, because that would change what the new environment trusts: +their `~/...` entries outside `~/.openclaw` now resolve under the environment +root, and migration prints a warning naming each such setting. Replace those +entries with absolute paths to keep trusting the original directories. In +these warnings, `[]` in a setting name stands for a list entry. The source home +is your home directory when the migrated directory is your own `~/.openclaw`, +through any path that leads to it; otherwise it is the directory that contains +the migrated `.openclaw` as you spelled it, or as it is stored when only that +path is named `.openclaw`, or your home directory for a state directory with +another name. Values under `~/.openclaw` already name the environment's copied +state and stay as written. Clone and archive import keep `~/...` values unchanged, because their source already expands `~` against an environment root. diff --git a/src/store/openclaw_config.rs b/src/store/openclaw_config.rs index 14c86dcb..1c7d7a74 100644 --- a/src/store/openclaw_config.rs +++ b/src/store/openclaw_config.rs @@ -232,7 +232,14 @@ pub(crate) fn rewrite_openclaw_config_for_migration( gateway_port: Option, sandbox_origin: Option<&str>, ) -> Result { - let kept = expand_migrated_home_relative_paths(target_paths, source_home)?; + let kept = expand_migrated_home_relative_paths( + target_paths, + &HomeExpansion { + home: source_home, + source_state_dir: source_state_root, + target_state_dir: &target_paths.state_dir, + }, + )?; rewrite_migrated_plugin_load_paths( target_paths, source_state_root, @@ -258,10 +265,10 @@ pub(crate) fn rewrite_openclaw_config_for_migration( /// although they may now resolve elsewhere. fn expand_migrated_home_relative_paths( target: &EnvPaths, - source_home: &Path, + expansion: &HomeExpansion<'_>, ) -> Result, String> { let mut kept = BTreeSet::new(); - if !source_home.is_absolute() { + if !expansion.home.is_absolute() { return Ok(kept); } let Some(effective) = load_effective_openclaw_config(&target.config_path)? else { @@ -276,7 +283,7 @@ fn expand_migrated_home_relative_paths( let mut rewritten = None; for mut scope in scopes { let mut candidate = value.clone(); - expand_home_relative_paths_in_scope(&mut candidate, &mut scope, source_home, &mut kept); + expand_home_relative_paths_in_scope(&mut candidate, &mut scope, expansion, &mut kept); if rewritten .as_ref() .is_some_and(|previous| previous != &candidate) @@ -295,15 +302,24 @@ fn expand_migrated_home_relative_paths( Ok(kept) } +/// The home `~` named in the plain install, and the state directory the +/// later relocation moves absolute values from and to. +struct HomeExpansion<'a> { + home: &'a Path, + source_state_dir: &'a Path, + target_state_dir: &'a Path, +} + /// Expands the home-relative path settings in a config fragment included at /// `scope`, and records the settings it keeps as written although they may /// now resolve elsewhere. `$include` directives keep their own relocation. fn expand_home_relative_paths_in_scope( value: &mut Value, scope: &mut Vec, - home: &Path, + expansion: &HomeExpansion<'_>, kept: &mut BTreeSet, ) -> bool { + let home = expansion.home; match value { Value::String(raw) => { let Some(rule) = home_relative_path_setting(scope) else { @@ -313,7 +329,9 @@ fn expand_home_relative_paths_in_scope( HomePathRule::Lexical { trims } => { (expand_home_relative_path(raw, home, trims), trims) } - HomePathRule::Physical => (expand_physical_home_relative_path(raw, home), false), + HomePathRule::Physical => { + (expand_physical_home_relative_path(raw, expansion), false) + } HomePathRule::TrustRoot => { let moves = expand_home_relative_path(raw, home, true).is_some() || has_home_relative_placeholder(raw, true); @@ -343,7 +361,7 @@ fn expand_home_relative_paths_in_scope( let mut changed = false; for (index, nested) in values.iter_mut().enumerate() { scope.push(ConfigSegment::Index(index)); - changed |= expand_home_relative_paths_in_scope(nested, scope, home, kept); + changed |= expand_home_relative_paths_in_scope(nested, scope, expansion, kept); scope.pop(); } changed @@ -352,7 +370,7 @@ fn expand_home_relative_paths_in_scope( let mut changed = false; for (key, nested) in values.iter_mut().filter(|(key, _)| *key != "$include") { scope.push(ConfigSegment::Key(key.clone())); - changed |= expand_home_relative_paths_in_scope(nested, scope, home, kept); + changed |= expand_home_relative_paths_in_scope(nested, scope, expansion, kept); scope.pop(); } changed @@ -572,12 +590,15 @@ fn expand_home_relative_path(raw: &str, home: &Path, trims: bool) -> Option Option { +/// which removing `..` here would change. A value under `~/.openclaw` with +/// no `..` already names the copied state directory and stays as written. +/// The later relocation of absolute state paths follows existing links as +/// the consumer does, but it steps over a missing directory the consumer +/// would reject. An entry it would move into the copied state although the +/// source path does not resolve stays as written, so it keeps resolving +/// through the copy of that missing directory. +fn expand_physical_home_relative_path(raw: &str, expansion: &HomeExpansion<'_>) -> Option { + let home = expansion.home; if !is_home_relative(raw) { return None; } @@ -585,13 +606,21 @@ fn expand_physical_home_relative_path(raw: &str, home: &Path) -> Option let mut literal_path = home.as_os_str().to_os_string(); literal_path.push(&literal[1..]); let literal_path = Path::new(&literal_path); - let expanded = format!("{}{}", home.to_str()?, &raw[1..]); if literal_path.starts_with(home.join(".openclaw")) && (!rest.is_empty() || !literal_path .components() - .any(|component| component == std::path::Component::ParentDir) - || fs::canonicalize(&expanded).is_err()) + .any(|component| component == std::path::Component::ParentDir)) + { + return None; + } + let expanded = format!("{}{}", home.to_str()?, &raw[1..]); + let mut relocated = expanded.clone(); + if rewrite_env_root_string( + &mut relocated, + expansion.source_state_dir, + expansion.target_state_dir, + ) && fs::canonicalize(&expanded).is_err() { return None; } @@ -3259,8 +3288,12 @@ mod tests { #[cfg(unix)] #[test] fn migration_home_expansion_keeps_link_traversal_for_physical_settings() { - let home = Path::new("/home/alice"); - let expand = |raw| expand_physical_home_relative_path(raw, home); + let expansion = HomeExpansion { + home: Path::new("/home/alice"), + source_state_dir: Path::new("/home/alice/.openclaw"), + target_state_dir: Path::new("/envs/copied/.openclaw"), + }; + let expand = |raw| expand_physical_home_relative_path(raw, &expansion); assert_eq!(expand("~/work").as_deref(), Some("/home/alice/work")); assert_eq!(expand("~").as_deref(), Some("/home/alice")); // `..` stays for the consumer to resolve after following `hop`. @@ -3268,9 +3301,17 @@ mod tests { expand("~/hop/../work").as_deref(), Some("/home/alice/hop/../work") ); - // Under `~/.openclaw`, `..` is spelled out only when it leads to an - // existing directory; this home does not exist. - assert_eq!(expand("~/.openclaw/../work"), None); + // `..` that leaves the state directory is not moved into the copy. + assert_eq!( + expand("~/.openclaw/../work").as_deref(), + Some("/home/alice/.openclaw/../work") + ); + assert_eq!( + expand("~/.openclaw/../../shared").as_deref(), + Some("/home/alice/.openclaw/../../shared") + ); + // The relocation would step over `missing`, which does not resolve. + assert_eq!(expand("~/.openclaw/missing/../wf"), None); assert_eq!( expand("~/${SUB}/work").as_deref(), Some("/home/alice/${SUB}/work") diff --git a/src/store/openclaw_workspaces.rs b/src/store/openclaw_workspaces.rs index 77b5a120..415a56a2 100644 --- a/src/store/openclaw_workspaces.rs +++ b/src/store/openclaw_workspaces.rs @@ -54,7 +54,12 @@ impl ConfigSegment { pub(crate) fn is(&self, name: &str) -> bool { match self { Self::Key(key) => key == name, - Self::Index(index) => index.to_string() == name, + // The decimal spelling of the index, without allocating one. + Self::Index(index) => { + name.bytes().all(|byte| byte.is_ascii_digit()) + && (name.len() == 1 || !name.starts_with('0')) + && name.parse() == Ok(*index) + } } } } diff --git a/tests/migrate_command_tests.rs b/tests/migrate_command_tests.rs index 7b96a438..8e83d810 100644 --- a/tests/migrate_command_tests.rs +++ b/tests/migrate_command_tests.rs @@ -5514,6 +5514,9 @@ fn migrate_and_adopt_expand_worker_folders_without_resolving_parent_traversal() // OpenClaw resolves `hop` before `..`, so `~/hop/../work` names // `~/data/work`, not `~/work`. std::os::unix::fs::symlink(home.join("data/deep"), home.join("hop")).unwrap(); + // `~/.openclaw/../../shared` names the missing `/shared` in the + // plain install; kept as written it would name this directory. + fs::create_dir_all(root.child("ocm-home/envs/shared")).unwrap(); let config = serde_json::json!({ "nodeHost": {"workerRuns": {"enabled": true, "folders": [ "~/work", @@ -5522,6 +5525,7 @@ fn migrate_and_adopt_expand_worker_folders_without_resolving_parent_traversal() "~/.openclaw/../work", // `missing` does not exist, so OpenClaw rejects this entry. "~/.openclaw/missing/../wf", + "~/.openclaw/../../shared", " ~/work", ]}} }); @@ -5553,6 +5557,7 @@ fn migrate_and_adopt_expand_worker_folders_without_resolving_parent_traversal() // Kept, so it still names a missing directory in the copy // instead of being resolved to the copied `wf`. "~/.openclaw/missing/../wf", + format!("{home_path}/.openclaw/../../shared"), // OpenClaw does not trim, so this is not home-relative. " ~/work", ]), @@ -5623,6 +5628,96 @@ fn migrate_and_adopt_keep_home_relative_trust_roots_and_name_them() { } } +#[test] +fn migrate_and_adopt_name_trust_roots_with_placeholders_under_the_state_dir() { + for adopt in [false, true] { + let root = TestDir::new("migrate-home-relative-trust-root-placeholders"); + let env = ocm_env(&root); + let home = root.child("home"); + let source = home.join(".openclaw"); + fs::create_dir_all(&source).unwrap(); + let config = serde_json::json!({ + "skills": {"load": {"allowSymlinkTargets": ["~/.openclaw/${SUB}/skills"]}}, + "security": {"installPolicy": {"exec": {"trustedDirs": [ + {"$include": "./trusted-dir.json"} + ]}}} + }); + support::write_text(&source.join("openclaw.json"), &config.to_string()); + support::write_text(&source.join("trusted-dir.json"), r#""~/.openclaw/${BIN}""#); + + let imported = run_ocm( + root.path(), + &env, + &migrate_or_adopt_args(adopt, "copied", &source), + ); + assert!(imported.status.success(), "{}", stderr(&imported)); + + let target = root.child("ocm-home/envs/copied/.openclaw"); + let copied: serde_json::Value = + serde_json::from_str(&fs::read_to_string(target.join("openclaw.json")).unwrap()) + .unwrap(); + assert_eq!( + copied["skills"]["load"]["allowSymlinkTargets"], + serde_json::json!(["~/.openclaw/${SUB}/skills"]), + "adopt={adopt}" + ); + assert_eq!( + fs::read_to_string(target.join("trusted-dir.json")).unwrap(), + r#""~/.openclaw/${BIN}""# + ); + let warnings = stderr(&imported); + for setting in [ + "skills.load.allowSymlinkTargets", + "security.installPolicy.exec.trustedDirs", + ] { + assert!( + warnings.lines().any(|line| line.starts_with(&format!( + "warning: env copied keeps the ~/ entries of trust root {setting}[] as written" + ))), + "adopt={adopt}: {warnings}" + ); + } + } +} + +#[test] +fn migrate_and_adopt_refuse_an_include_shared_by_a_path_list_and_a_trust_root() { + for adopt in [false, true] { + let root = TestDir::new("migrate-home-relative-shared-list-include"); + let env = ocm_env(&root); + let home = root.child("home"); + let source = home.join(".openclaw"); + fs::create_dir_all(&source).unwrap(); + // One file cannot be spelled out for `extraDirs` and kept for the + // trust root, so migration refuses instead of moving either silently. + let config = serde_json::json!({ + "skills": {"load": { + "extraDirs": [{"$include": "./shared-dir.json"}], + "allowSymlinkTargets": [{"$include": "./shared-dir.json"}] + }} + }); + support::write_text(&source.join("openclaw.json"), &config.to_string()); + support::write_text(&source.join("shared-dir.json"), r#""~/shared""#); + + let imported = run_ocm( + root.path(), + &env, + &migrate_or_adopt_args(adopt, "copied", &source), + ); + assert!(!imported.status.success(), "adopt={adopt}"); + assert!( + stderr(&imported) + .contains("cannot safely expand home-relative paths in shared OpenClaw include"), + "adopt={adopt}: {}", + stderr(&imported) + ); + assert_eq!( + fs::read_to_string(source.join("shared-dir.json")).unwrap(), + r#""~/shared""# + ); + } +} + #[test] fn migrate_and_adopt_do_not_name_trust_roots_that_keep_their_meaning() { for adopt in [false, true] { From 9b373629a5bdca7aba6e913eeb26606d7496f40e Mon Sep 17 00:00:00 2001 From: Shakker Date: Mon, 2 Nov 2026 12:03:06 +0000 Subject: [PATCH 5/8] fix: keep and name worker folders whose .. leads into ~/.openclaw Migration can change the copied state's links and directories, for example when it imports a linked workspace, so a worker folder whose `..` or placeholder leads into `~/.openclaw` cannot be spelled out to the same target. It now stays as written and migration prints a warning naming nodeHost.workerRuns.folders. Entries the state relocation leaves alone are still spelled out against the source home. --- CHANGELOG.md | 2 +- docs/USAGE.md | 9 ++- src/cli/migrate.rs | 5 +- src/store/openclaw_config.rs | 144 +++++++++++++++++++++------------ tests/migrate_command_tests.rs | 60 +++++++++++++- 5 files changed, 161 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 664d3f75..b62461e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ All notable changes to OCM are documented here. - Find the installed npm launcher by default when building and installing runtimes on Windows. - Remove the hidden staging file and SQLite snapshot copies when `ocm env export` is stopped by SIGINT, SIGTERM or SIGHUP, and exit with status 130. Signals ignored at startup, as under `nohup`, stay ignored. - Keep `~/...` config paths outside `~/.openclaw`, such as local plugin, extra skills, agent directory and session or cron store paths, pointing at the same location after `migrate` and `adopt import`, instead of letting them resolve under the new environment root. Other config text is left unchanged. -- Also keep `~/...` values in `nodeHost.workerRuns.folders`, in includes placed at a list position and in values containing a `${VAR}` placeholder outside `~/.openclaw` pointing at the same location after `migrate` and `adopt import`. Home-relative trust roots in `skills.load.allowSymlinkTargets` and `security.installPolicy.exec.trustedDirs`, and `~/.openclaw/...` values containing a placeholder, stay as written, and migration now prints a warning naming each such setting, because they may resolve under the new environment root. +- Also keep `~/...` values in `nodeHost.workerRuns.folders`, in includes placed at a list position and in values containing a `${VAR}` placeholder outside `~/.openclaw` pointing at the same location after `migrate` and `adopt import`. Home-relative trust roots in `skills.load.allowSymlinkTargets` and `security.installPolicy.exec.trustedDirs`, `~/.openclaw/...` values containing a placeholder, and worker folders whose `..` leads into `~/.openclaw` stay as written, and migration now prints a warning naming each such setting, because they may resolve differently in the new environment. ## 0.3.8 - 2026-10-29 diff --git a/docs/USAGE.md b/docs/USAGE.md index aa6e3b7b..af90c938 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -1298,10 +1298,11 @@ value spelled from your home, so a placeholder that itself leads into setting, because its placeholder may still lead outside `~/.openclaw`. `nodeHost.workerRuns.folders` entries, which OpenClaw resolves through symlinks, only have their leading `~` replaced, so a link before `..` still -decides where they lead; an entry that then names a directory inside -`~/.openclaw` moves to the environment's copy like any other state path, and -one that would only reach the copy by stepping over a directory missing from -`~/.openclaw` stays as written, so OpenClaw keeps rejecting it. The trust roots +decides where they lead. An entry whose `..` or placeholder leads into +`~/.openclaw` stays as written instead, and migration prints a warning naming +its setting: OpenClaw then resolves it through the environment's copy of +`~/.openclaw`, whose links and directories can differ from the source, for +example after a linked workspace is imported. The trust roots `skills.load.allowSymlinkTargets` and `security.installPolicy.exec.trustedDirs` are not rewritten, because that would change what the new environment trusts: their `~/...` entries outside `~/.openclaw` now resolve under the environment diff --git a/src/cli/migrate.rs b/src/cli/migrate.rs index 66eee1bc..ac46ece3 100644 --- a/src/cli/migrate.rs +++ b/src/cli/migrate.rs @@ -263,7 +263,10 @@ impl Cli { "warning: env {env_name} keeps the ~/ entries of trust root {setting} as written; they now resolve under the env root instead of the source home, so OCM did not carry that trust over. Replace them with absolute paths to keep trusting the original directories" ), crate::store::KeptHomeRelativeReason::Placeholder => format!( - "warning: env {env_name} keeps the ~/.openclaw entries of {setting} that contain a ${{VAR}} placeholder as written; if a placeholder leads outside ~/.openclaw, the entry now resolves under the env root instead of the source home. Replace such entries with absolute paths" + "warning: env {env_name} keeps the ~/ entries of {setting} that contain a ${{VAR}} placeholder and point into ~/.openclaw as written; if a placeholder leads them outside ~/.openclaw, they now resolve under the env root instead of the source home. Replace such entries with absolute paths" + ), + crate::store::KeptHomeRelativeReason::StateDirTraversal => format!( + "warning: env {env_name} keeps the ~/ entries of {setting} whose .. leads into ~/.openclaw as written; OpenClaw now resolves them through the env's copy of ~/.openclaw, whose links and directories can differ from the source, so they may name a different directory than before. Check them and replace them with absolute paths if needed" ), }); } diff --git a/src/store/openclaw_config.rs b/src/store/openclaw_config.rs index 1c7d7a74..a649e2ec 100644 --- a/src/store/openclaw_config.rs +++ b/src/store/openclaw_config.rs @@ -59,6 +59,10 @@ pub(crate) enum KeptHomeRelativeReason { /// A value under `~/.openclaw` with a `${VAR}` placeholder, which may /// lead outside the state directory once substituted. Placeholder, + /// A value resolved through links whose `..` passes through + /// `~/.openclaw`, where the copied state's links and directories decide + /// where it leads. + StateDirTraversal, } pub(crate) fn audit_openclaw_config( @@ -325,37 +329,46 @@ fn expand_home_relative_paths_in_scope( let Some(rule) = home_relative_path_setting(scope) else { return false; }; - let (expanded, trims) = match rule { + let expanded = match rule { HomePathRule::Lexical { trims } => { - (expand_home_relative_path(raw, home, trims), trims) + match expand_home_relative_path(raw, home, trims) { + Some(expanded) => Ok(expanded), + // A placeholder under `~/.openclaw` can still lead outside it. + None if has_home_relative_placeholder(raw, trims) => { + Err(KeptHomeRelativeReason::Placeholder) + } + None => return false, + } } HomePathRule::Physical => { - (expand_physical_home_relative_path(raw, expansion), false) + match expand_physical_home_relative_path(raw, expansion) { + Ok(Some(expanded)) => Ok(expanded), + Ok(None) => return false, + Err(reason) => Err(reason), + } } HomePathRule::TrustRoot => { - let moves = expand_home_relative_path(raw, home, true).is_some() - || has_home_relative_placeholder(raw, true); - if moves { - kept.insert(KeptHomeRelativeSetting { - setting: config_setting_name(scope), - reason: KeptHomeRelativeReason::TrustRoot, - }); + if expand_home_relative_path(raw, home, true).is_none() + && !has_home_relative_placeholder(raw, true) + { + return false; } - return false; + Err(KeptHomeRelativeReason::TrustRoot) } }; - let Some(expanded) = expanded else { - // A placeholder under `~/.openclaw` can still lead outside it. - if has_home_relative_placeholder(raw, trims) { + match expanded { + Ok(expanded) => { + *raw = expanded; + true + } + Err(reason) => { kept.insert(KeptHomeRelativeSetting { setting: config_setting_name(scope), - reason: KeptHomeRelativeReason::Placeholder, + reason, }); + false } - return false; - }; - *raw = expanded; - true + } } Value::Array(values) => { let mut changed = false; @@ -590,41 +603,60 @@ fn expand_home_relative_path(raw: &str, home: &Path, trims: bool) -> Option) -> Option { +/// which removing `..` here would change. Returns `Ok(None)` for a value +/// that keeps its meaning as written, such as one under `~/.openclaw` with +/// no `..`, which already names the copied state directory. The later +/// relocation of absolute state paths would move a spelled-out entry whose +/// `..` or placeholder leads into the state directory to the copied state, +/// whose links and directories migration may have changed (an imported +/// workspace, for example), so it would no longer follow the source's links. +/// Such an entry stays as written and is reported instead, as is a +/// placeholder value under `~/.openclaw`. +fn expand_physical_home_relative_path( + raw: &str, + expansion: &HomeExpansion<'_>, +) -> Result, KeptHomeRelativeReason> { let home = expansion.home; if !is_home_relative(raw) { - return None; + return Ok(None); } let (literal, rest) = split_before_placeholder(raw); let mut literal_path = home.as_os_str().to_os_string(); literal_path.push(&literal[1..]); let literal_path = Path::new(&literal_path); - if literal_path.starts_with(home.join(".openclaw")) - && (!rest.is_empty() - || !literal_path - .components() - .any(|component| component == std::path::Component::ParentDir)) - { - return None; + let has_parent = |path: &Path| { + path.components() + .any(|component| component == std::path::Component::ParentDir) + }; + if literal_path.starts_with(home.join(".openclaw")) { + if !rest.is_empty() { + return Err(KeptHomeRelativeReason::Placeholder); + } + if !has_parent(literal_path) { + return Ok(None); + } } - let expanded = format!("{}{}", home.to_str()?, &raw[1..]); + let Some(home_text) = home.to_str() else { + return Ok(None); + }; + let expanded = format!("{home_text}{}", &raw[1..]); + // Probe a scratch copy with the same relocation that runs later: an + // entry it leaves alone keeps following the source's own links. let mut relocated = expanded.clone(); - if rewrite_env_root_string( - &mut relocated, - expansion.source_state_dir, - expansion.target_state_dir, - ) && fs::canonicalize(&expanded).is_err() + if (has_parent(Path::new(&expanded)) || !rest.is_empty()) + && rewrite_env_root_string( + &mut relocated, + expansion.source_state_dir, + expansion.target_state_dir, + ) { - return None; + return Err(if rest.is_empty() { + KeptHomeRelativeReason::StateDirTraversal + } else { + KeptHomeRelativeReason::Placeholder + }); } - Some(expanded) + Ok(Some(expanded)) } /// Moves a Gateway port that a copied `$include` supplies to `gateway_port`, @@ -3293,7 +3325,7 @@ mod tests { source_state_dir: Path::new("/home/alice/.openclaw"), target_state_dir: Path::new("/envs/copied/.openclaw"), }; - let expand = |raw| expand_physical_home_relative_path(raw, &expansion); + let expand = |raw| expand_physical_home_relative_path(raw, &expansion).unwrap(); assert_eq!(expand("~/work").as_deref(), Some("/home/alice/work")); assert_eq!(expand("~").as_deref(), Some("/home/alice")); // `..` stays for the consumer to resolve after following `hop`. @@ -3310,22 +3342,30 @@ mod tests { expand("~/.openclaw/../../shared").as_deref(), Some("/home/alice/.openclaw/../../shared") ); - // The relocation would step over `missing`, which does not resolve. - assert_eq!(expand("~/.openclaw/missing/../wf"), None); + // The relocation would resolve these through the copied state. + for traversal in ["~/.openclaw/missing/../wf", "~/.openclaw/a/../wf"] { + assert_eq!( + expand_physical_home_relative_path(traversal, &expansion), + Err(KeptHomeRelativeReason::StateDirTraversal), + "{traversal}" + ); + } assert_eq!( expand("~/${SUB}/work").as_deref(), Some("/home/alice/${SUB}/work") ); // The consumer does not trim. assert_eq!(expand(" ~/work"), None); - for copied in [ - "~/.openclaw", - "~/.openclaw/wf", - "~/.openclaw/${SUB}", - "~/.openclaw/a/../${SUB}", - ] { + for copied in ["~/.openclaw", "~/.openclaw/wf"] { assert_eq!(expand(copied), None, "{copied}"); } + for placeholder in ["~/.openclaw/${SUB}", "~/.openclaw/a/../${SUB}"] { + assert_eq!( + expand_physical_home_relative_path(placeholder, &expansion), + Err(KeptHomeRelativeReason::Placeholder), + "{placeholder}" + ); + } assert_eq!( expand("~/.openclawish").as_deref(), Some("/home/alice/.openclawish") diff --git a/tests/migrate_command_tests.rs b/tests/migrate_command_tests.rs index 8e83d810..a176e5e7 100644 --- a/tests/migrate_command_tests.rs +++ b/tests/migrate_command_tests.rs @@ -5489,7 +5489,7 @@ fn migrate_and_adopt_expand_home_relative_paths_with_placeholders_before_substit warnings .lines() .filter(|line| line.starts_with( - "warning: env copied keeps the ~/.openclaw entries of skills.load.extraDirs[] that contain a ${VAR} placeholder" + "warning: env copied keeps the ~/ entries of skills.load.extraDirs[] that contain a ${VAR} placeholder and point into ~/.openclaw" )) .count(), 1, @@ -5567,6 +5567,64 @@ fn migrate_and_adopt_expand_worker_folders_without_resolving_parent_traversal() fs::canonicalize(folders[1].as_str().unwrap()).unwrap(), fs::canonicalize(home.join("data/work")).unwrap() ); + let warnings = stderr(&imported); + assert_eq!( + warnings + .lines() + .filter(|line| line.starts_with( + "warning: env copied keeps the ~/ entries of nodeHost.workerRuns.folders[] whose .. leads into ~/.openclaw as written" + )) + .count(), + 1, + "adopt={adopt}: {warnings}" + ); + } +} + +#[cfg(unix)] +#[test] +fn migrate_and_adopt_keep_worker_folders_resolved_through_an_imported_workspace() { + for adopt in [false, true] { + let root = TestDir::new("migrate-home-relative-worker-workspace"); + let env = ocm_env(&root); + let home = root.child("home"); + let source = home.join(".openclaw"); + fs::create_dir_all(source.join("wf")).unwrap(); + fs::create_dir_all(home.join("external/deep")).unwrap(); + fs::create_dir_all(home.join("external/wf")).unwrap(); + // OpenClaw resolves `workspace/../wf` to `~/external/wf`. Migration + // imports the linked workspace, so the copied state no longer has + // that link and the same entry would name the copied `wf`. + std::os::unix::fs::symlink(home.join("external/deep"), source.join("workspace")).unwrap(); + let config = serde_json::json!({ + "agents": {"defaults": {"workspace": "~/.openclaw/workspace"}}, + "nodeHost": {"workerRuns": {"folders": ["~/.openclaw/workspace/../wf"]}} + }); + support::write_text(&source.join("openclaw.json"), &config.to_string()); + + let imported = run_ocm( + root.path(), + &env, + &migrate_or_adopt_args(adopt, "copied", &source), + ); + assert!(imported.status.success(), "{}", stderr(&imported)); + + let target = root.child("ocm-home/envs/copied/.openclaw"); + let copied: serde_json::Value = + serde_json::from_str(&fs::read_to_string(target.join("openclaw.json")).unwrap()) + .unwrap(); + assert_eq!( + copied["nodeHost"]["workerRuns"]["folders"], + serde_json::json!(["~/.openclaw/workspace/../wf"]), + "adopt={adopt}" + ); + assert!( + stderr(&imported).lines().any(|line| line.starts_with( + "warning: env copied keeps the ~/ entries of nodeHost.workerRuns.folders[] whose .. leads into ~/.openclaw as written" + )), + "adopt={adopt}: {}", + stderr(&imported) + ); } } From 41d5303b6d43df1b7e4deec8b0bd9c84e1f747c7 Mon Sep 17 00:00:00 2001 From: Shakker Date: Mon, 2 Nov 2026 12:14:37 +0000 Subject: [PATCH 6/8] fix: keep worker folders as written and name them after migrate Migration can change the links and directories a worker folder resolves through, and OpenClaw uses that folder as a permission boundary, so `nodeHost.workerRuns.folders` entries are no longer spelled out or relocated. Every home-relative entry stays exactly as on main and migration prints a warning naming the setting. The limit is documented in USAGE and stays open in #972. --- CHANGELOG.md | 2 +- docs/USAGE.md | 38 +++---- src/cli/migrate.rs | 4 +- src/store/openclaw_config.rs | 175 +++++---------------------------- tests/migrate_command_tests.rs | 89 +++++------------ 5 files changed, 69 insertions(+), 239 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b62461e0..71483b58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ All notable changes to OCM are documented here. - Find the installed npm launcher by default when building and installing runtimes on Windows. - Remove the hidden staging file and SQLite snapshot copies when `ocm env export` is stopped by SIGINT, SIGTERM or SIGHUP, and exit with status 130. Signals ignored at startup, as under `nohup`, stay ignored. - Keep `~/...` config paths outside `~/.openclaw`, such as local plugin, extra skills, agent directory and session or cron store paths, pointing at the same location after `migrate` and `adopt import`, instead of letting them resolve under the new environment root. Other config text is left unchanged. -- Also keep `~/...` values in `nodeHost.workerRuns.folders`, in includes placed at a list position and in values containing a `${VAR}` placeholder outside `~/.openclaw` pointing at the same location after `migrate` and `adopt import`. Home-relative trust roots in `skills.load.allowSymlinkTargets` and `security.installPolicy.exec.trustedDirs`, `~/.openclaw/...` values containing a placeholder, and worker folders whose `..` leads into `~/.openclaw` stay as written, and migration now prints a warning naming each such setting, because they may resolve differently in the new environment. +- Also keep `~/...` values in includes placed at a list position and in values containing a `${VAR}` placeholder outside `~/.openclaw` pointing at the same location after `migrate` and `adopt import`. Home-relative trust roots in `skills.load.allowSymlinkTargets` and `security.installPolicy.exec.trustedDirs`, `~/.openclaw/...` values containing a placeholder, and `nodeHost.workerRuns.folders` entries stay as written, and migration now prints a warning naming each such setting, because they may resolve differently in the new environment. ## 0.3.8 - 2026-10-29 diff --git a/docs/USAGE.md b/docs/USAGE.md index af90c938..f018a2c8 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -1275,8 +1275,8 @@ still apply. Migration and adoption also spell out home-relative config paths. A plain install expands `~` against its home, but the new environment runs with `OPENCLAW_HOME` set to its root, so OpenClaw expands `~` against the environment -root instead. Every setting OpenClaw expands `~` in that names a location outside -`~/.openclaw`, such as a local plugin in `plugins.load.paths`, a +root instead. Every setting OpenClaw expands `~` in that names a location +outside `~/.openclaw`, such as a local plugin in `plugins.load.paths`, a `skills.load.extraDirs` entry, an agent directory or a session or cron store, is therefore rewritten in `openclaw.json` and its copied includes to the absolute path it named in the source home. These are the settings OpenClaw's runtime @@ -1289,27 +1289,27 @@ handling and lexical `.` and `..` resolution as its OpenClaw consumer. Settings OpenClaw expands against the OS home keep their meaning without a rewrite, because the environment keeps your `HOME`. Other strings, such as message text, stay as written. Path values in an include placed at a list position are -rewritten like any other. In a value containing a `${VAR}` placeholder, only -the directories before the first placeholder are resolved; OpenClaw -substitutes the placeholder and resolves the rest, as it would for the same -value spelled from your home, so a placeholder that itself leads into -`~/.openclaw` names the source's state directory. A placeholder value under -`~/.openclaw` stays as written, and migration prints a warning naming its -setting, because its placeholder may still lead outside `~/.openclaw`. -`nodeHost.workerRuns.folders` entries, which OpenClaw resolves through -symlinks, only have their leading `~` replaced, so a link before `..` still -decides where they lead. An entry whose `..` or placeholder leads into -`~/.openclaw` stays as written instead, and migration prints a warning naming -its setting: OpenClaw then resolves it through the environment's copy of -`~/.openclaw`, whose links and directories can differ from the source, for -example after a linked workspace is imported. The trust roots +rewritten like any other. In a value containing a `${VAR}` placeholder, only the +directories before the first placeholder are resolved; OpenClaw substitutes the +placeholder and resolves the rest, as it would for the same value spelled from +your home, so a placeholder that itself leads into `~/.openclaw` names the +source's state directory. A placeholder value under `~/.openclaw` stays as +written, and migration prints a warning naming its setting, because its +placeholder may still lead outside `~/.openclaw`. `nodeHost.workerRuns.folders` +entries are not rewritten: OpenClaw resolves them to real paths, through +symlinks, and grants workers access to the result, and migration can change the +links and directories they resolve through (for example when it imports a linked +workspace), so no spelling is known to keep the same folder. Their `~/...` +entries now resolve under the environment root, and migration prints a warning +naming the setting. Replace them with absolute paths to keep the original +folders ([#972](https://github.com/openclaw/ocm/issues/972)). The trust roots `skills.load.allowSymlinkTargets` and `security.installPolicy.exec.trustedDirs` are not rewritten, because that would change what the new environment trusts: their `~/...` entries outside `~/.openclaw` now resolve under the environment root, and migration prints a warning naming each such setting. Replace those -entries with absolute paths to keep trusting the original directories. In -these warnings, `[]` in a setting name stands for a list entry. The source home -is your home directory when the migrated directory is your own `~/.openclaw`, +entries with absolute paths to keep trusting the original directories. In these +warnings, `[]` in a setting name stands for a list entry. The source home is +your home directory when the migrated directory is your own `~/.openclaw`, through any path that leads to it; otherwise it is the directory that contains the migrated `.openclaw` as you spelled it, or as it is stored when only that path is named `.openclaw`, or your home directory for a state directory with diff --git a/src/cli/migrate.rs b/src/cli/migrate.rs index ac46ece3..6f7b62f2 100644 --- a/src/cli/migrate.rs +++ b/src/cli/migrate.rs @@ -265,8 +265,8 @@ impl Cli { crate::store::KeptHomeRelativeReason::Placeholder => format!( "warning: env {env_name} keeps the ~/ entries of {setting} that contain a ${{VAR}} placeholder and point into ~/.openclaw as written; if a placeholder leads them outside ~/.openclaw, they now resolve under the env root instead of the source home. Replace such entries with absolute paths" ), - crate::store::KeptHomeRelativeReason::StateDirTraversal => format!( - "warning: env {env_name} keeps the ~/ entries of {setting} whose .. leads into ~/.openclaw as written; OpenClaw now resolves them through the env's copy of ~/.openclaw, whose links and directories can differ from the source, so they may name a different directory than before. Check them and replace them with absolute paths if needed" + crate::store::KeptHomeRelativeReason::WorkerFolder => format!( + "warning: env {env_name} keeps the ~/ entries of {setting} as written; OpenClaw now resolves them against the env root, so workers may be allowed or refused different folders than before. Replace them with absolute paths to keep the original folders" ), }); } diff --git a/src/store/openclaw_config.rs b/src/store/openclaw_config.rs index a649e2ec..453046fa 100644 --- a/src/store/openclaw_config.rs +++ b/src/store/openclaw_config.rs @@ -59,10 +59,11 @@ pub(crate) enum KeptHomeRelativeReason { /// A value under `~/.openclaw` with a `${VAR}` placeholder, which may /// lead outside the state directory once substituted. Placeholder, - /// A value resolved through links whose `..` passes through - /// `~/.openclaw`, where the copied state's links and directories decide - /// where it leads. - StateDirTraversal, + /// A node worker folder. OpenClaw resolves it to a real path and grants + /// workers access to it, and migration can change the links and + /// directories it resolves through, so no spelling is known to keep the + /// same folder. + WorkerFolder, } pub(crate) fn audit_openclaw_config( @@ -236,14 +237,7 @@ pub(crate) fn rewrite_openclaw_config_for_migration( gateway_port: Option, sandbox_origin: Option<&str>, ) -> Result { - let kept = expand_migrated_home_relative_paths( - target_paths, - &HomeExpansion { - home: source_home, - source_state_dir: source_state_root, - target_state_dir: &target_paths.state_dir, - }, - )?; + let kept = expand_migrated_home_relative_paths(target_paths, source_home)?; rewrite_migrated_plugin_load_paths( target_paths, source_state_root, @@ -269,10 +263,10 @@ pub(crate) fn rewrite_openclaw_config_for_migration( /// although they may now resolve elsewhere. fn expand_migrated_home_relative_paths( target: &EnvPaths, - expansion: &HomeExpansion<'_>, + source_home: &Path, ) -> Result, String> { let mut kept = BTreeSet::new(); - if !expansion.home.is_absolute() { + if !source_home.is_absolute() { return Ok(kept); } let Some(effective) = load_effective_openclaw_config(&target.config_path)? else { @@ -287,7 +281,7 @@ fn expand_migrated_home_relative_paths( let mut rewritten = None; for mut scope in scopes { let mut candidate = value.clone(); - expand_home_relative_paths_in_scope(&mut candidate, &mut scope, expansion, &mut kept); + expand_home_relative_paths_in_scope(&mut candidate, &mut scope, source_home, &mut kept); if rewritten .as_ref() .is_some_and(|previous| previous != &candidate) @@ -306,24 +300,15 @@ fn expand_migrated_home_relative_paths( Ok(kept) } -/// The home `~` named in the plain install, and the state directory the -/// later relocation moves absolute values from and to. -struct HomeExpansion<'a> { - home: &'a Path, - source_state_dir: &'a Path, - target_state_dir: &'a Path, -} - /// Expands the home-relative path settings in a config fragment included at /// `scope`, and records the settings it keeps as written although they may /// now resolve elsewhere. `$include` directives keep their own relocation. fn expand_home_relative_paths_in_scope( value: &mut Value, scope: &mut Vec, - expansion: &HomeExpansion<'_>, + home: &Path, kept: &mut BTreeSet, ) -> bool { - let home = expansion.home; match value { Value::String(raw) => { let Some(rule) = home_relative_path_setting(scope) else { @@ -340,12 +325,12 @@ fn expand_home_relative_paths_in_scope( None => return false, } } - HomePathRule::Physical => { - match expand_physical_home_relative_path(raw, expansion) { - Ok(Some(expanded)) => Ok(expanded), - Ok(None) => return false, - Err(reason) => Err(reason), + HomePathRule::WorkerFolder => { + // OpenClaw does not trim these. + if !is_home_relative(raw) { + return false; } + Err(KeptHomeRelativeReason::WorkerFolder) } HomePathRule::TrustRoot => { if expand_home_relative_path(raw, home, true).is_none() @@ -374,7 +359,7 @@ fn expand_home_relative_paths_in_scope( let mut changed = false; for (index, nested) in values.iter_mut().enumerate() { scope.push(ConfigSegment::Index(index)); - changed |= expand_home_relative_paths_in_scope(nested, scope, expansion, kept); + changed |= expand_home_relative_paths_in_scope(nested, scope, home, kept); scope.pop(); } changed @@ -383,7 +368,7 @@ fn expand_home_relative_paths_in_scope( let mut changed = false; for (key, nested) in values.iter_mut().filter(|(key, _)| *key != "$include") { scope.push(ConfigSegment::Key(key.clone())); - changed |= expand_home_relative_paths_in_scope(nested, scope, expansion, kept); + changed |= expand_home_relative_paths_in_scope(nested, scope, home, kept); scope.pop(); } changed @@ -398,9 +383,9 @@ enum HomePathRule { /// The leading `~` is replaced, after trimming when `trims` is set, and /// the path is resolved lexically. Lexical { trims: bool }, - /// The leading `~` is replaced and the path is resolved through links - /// (`realpath`), so a link before `..` decides where it leads. - Physical, + /// A node worker folder, resolved to a real path and used as a + /// permission boundary. It stays as written and is reported. + WorkerFolder, /// A trust root. Spelling it out would change what the environment /// trusts, so it stays as written and is reported. TrustRoot, @@ -463,10 +448,11 @@ const CONSUMER_HOME_PATH_SETTINGS: &[(&[&str], HomePathRule)] = &[ HomePathRule::Lexical { trims: true }, ), // src/node-host/node-worker-device-folder.ts: `expandHomePrefix`, then - // `fs.realpathSync.native`, without trimming. + // `fs.realpathSync.native`, without trimming, to decide which folders + // workers may use. ( &["nodeHost", "workerRuns", "folders", "[]"], - HomePathRule::Physical, + HomePathRule::WorkerFolder, ), // src/skills/loading/symlink-targets.ts and src/security/install-policy.ts ( @@ -601,64 +587,6 @@ fn expand_home_relative_path(raw: &str, home: &Path, trims: bool) -> Option, -) -> Result, KeptHomeRelativeReason> { - let home = expansion.home; - if !is_home_relative(raw) { - return Ok(None); - } - let (literal, rest) = split_before_placeholder(raw); - let mut literal_path = home.as_os_str().to_os_string(); - literal_path.push(&literal[1..]); - let literal_path = Path::new(&literal_path); - let has_parent = |path: &Path| { - path.components() - .any(|component| component == std::path::Component::ParentDir) - }; - if literal_path.starts_with(home.join(".openclaw")) { - if !rest.is_empty() { - return Err(KeptHomeRelativeReason::Placeholder); - } - if !has_parent(literal_path) { - return Ok(None); - } - } - let Some(home_text) = home.to_str() else { - return Ok(None); - }; - let expanded = format!("{home_text}{}", &raw[1..]); - // Probe a scratch copy with the same relocation that runs later: an - // entry it leaves alone keeps following the source's own links. - let mut relocated = expanded.clone(); - if (has_parent(Path::new(&expanded)) || !rest.is_empty()) - && rewrite_env_root_string( - &mut relocated, - expansion.source_state_dir, - expansion.target_state_dir, - ) - { - return Err(if rest.is_empty() { - KeptHomeRelativeReason::StateDirTraversal - } else { - KeptHomeRelativeReason::Placeholder - }); - } - Ok(Some(expanded)) -} - /// Moves a Gateway port that a copied `$include` supplies to `gateway_port`, /// with the MCP app sandbox port and loopback origin that follow it in /// `config`, the target's root config after its include directives point at @@ -3278,7 +3206,7 @@ mod tests { ); assert_eq!( setting(&["nodeHost", "workerRuns", "folders", "[]"]), - Some(HomePathRule::Physical) + Some(HomePathRule::WorkerFolder) ); assert_eq!( setting(&["skills", "load", "allowSymlinkTargets", "[]"]), @@ -3317,61 +3245,6 @@ mod tests { } } - #[cfg(unix)] - #[test] - fn migration_home_expansion_keeps_link_traversal_for_physical_settings() { - let expansion = HomeExpansion { - home: Path::new("/home/alice"), - source_state_dir: Path::new("/home/alice/.openclaw"), - target_state_dir: Path::new("/envs/copied/.openclaw"), - }; - let expand = |raw| expand_physical_home_relative_path(raw, &expansion).unwrap(); - assert_eq!(expand("~/work").as_deref(), Some("/home/alice/work")); - assert_eq!(expand("~").as_deref(), Some("/home/alice")); - // `..` stays for the consumer to resolve after following `hop`. - assert_eq!( - expand("~/hop/../work").as_deref(), - Some("/home/alice/hop/../work") - ); - // `..` that leaves the state directory is not moved into the copy. - assert_eq!( - expand("~/.openclaw/../work").as_deref(), - Some("/home/alice/.openclaw/../work") - ); - assert_eq!( - expand("~/.openclaw/../../shared").as_deref(), - Some("/home/alice/.openclaw/../../shared") - ); - // The relocation would resolve these through the copied state. - for traversal in ["~/.openclaw/missing/../wf", "~/.openclaw/a/../wf"] { - assert_eq!( - expand_physical_home_relative_path(traversal, &expansion), - Err(KeptHomeRelativeReason::StateDirTraversal), - "{traversal}" - ); - } - assert_eq!( - expand("~/${SUB}/work").as_deref(), - Some("/home/alice/${SUB}/work") - ); - // The consumer does not trim. - assert_eq!(expand(" ~/work"), None); - for copied in ["~/.openclaw", "~/.openclaw/wf"] { - assert_eq!(expand(copied), None, "{copied}"); - } - for placeholder in ["~/.openclaw/${SUB}", "~/.openclaw/a/../${SUB}"] { - assert_eq!( - expand_physical_home_relative_path(placeholder, &expansion), - Err(KeptHomeRelativeReason::Placeholder), - "{placeholder}" - ); - } - assert_eq!( - expand("~/.openclawish").as_deref(), - Some("/home/alice/.openclawish") - ); - } - #[test] fn migration_home_expansion_types_include_scopes_from_traversal() { let root = tempfile::tempdir().unwrap(); diff --git a/tests/migrate_command_tests.rs b/tests/migrate_command_tests.rs index a176e5e7..0845501d 100644 --- a/tests/migrate_command_tests.rs +++ b/tests/migrate_command_tests.rs @@ -5187,9 +5187,10 @@ fn migrate_and_adopt_keep_home_relative_paths_outside_the_state_dir_pointing_at_ copied["hooks"]["internal"]["load"]["extraDirs"], serde_json::json!([format!("{home_path}/hooks")]) ); + // Kept as written and named in a warning; see the worker-folder test. assert_eq!( copied["nodeHost"]["workerRuns"]["folders"], - serde_json::json!([format!("{home_path}/work")]) + serde_json::json!(["~/work"]) ); // Expanded against the OS home, which the environment keeps. assert_eq!( @@ -5501,7 +5502,7 @@ fn migrate_and_adopt_expand_home_relative_paths_with_placeholders_before_substit #[cfg(unix)] #[test] -fn migrate_and_adopt_expand_worker_folders_without_resolving_parent_traversal() { +fn migrate_and_adopt_keep_worker_folders_and_name_them() { for adopt in [false, true] { let root = TestDir::new("migrate-home-relative-worker-folders"); let env = ocm_env(&root); @@ -5509,25 +5510,20 @@ fn migrate_and_adopt_expand_worker_folders_without_resolving_parent_traversal() let source = home.join(".openclaw"); fs::create_dir_all(source.join("wf")).unwrap(); fs::create_dir_all(home.join("data/deep")).unwrap(); - fs::create_dir_all(home.join("data/work")).unwrap(); fs::create_dir_all(home.join("work")).unwrap(); - // OpenClaw resolves `hop` before `..`, so `~/hop/../work` names - // `~/data/work`, not `~/work`. std::os::unix::fs::symlink(home.join("data/deep"), home.join("hop")).unwrap(); - // `~/.openclaw/../../shared` names the missing `/shared` in the - // plain install; kept as written it would name this directory. - fs::create_dir_all(root.child("ocm-home/envs/shared")).unwrap(); + let home_path = home.to_str().unwrap(); + let folders = serde_json::json!([ + "~/work", + "~/hop/../work", + "~/.openclaw/wf", + "~/${SUB}/work", + // OpenClaw does not trim, so this is not home-relative. + " ~/work", + format!("{home_path}/work"), + ]); let config = serde_json::json!({ - "nodeHost": {"workerRuns": {"enabled": true, "folders": [ - "~/work", - "~/hop/../work", - "~/.openclaw/wf", - "~/.openclaw/../work", - // `missing` does not exist, so OpenClaw rejects this entry. - "~/.openclaw/missing/../wf", - "~/.openclaw/../../shared", - " ~/work", - ]}} + "nodeHost": {"workerRuns": {"enabled": true, "folders": folders}} }); support::write_text(&source.join("openclaw.json"), &config.to_string()); @@ -5542,37 +5538,18 @@ fn migrate_and_adopt_expand_worker_folders_without_resolving_parent_traversal() let copied: serde_json::Value = serde_json::from_str(&fs::read_to_string(target.join("openclaw.json")).unwrap()) .unwrap(); - let home_path = home.to_str().unwrap(); - let folders = &copied["nodeHost"]["workerRuns"]["folders"]; + // OpenClaw resolves these to real paths and grants workers access to + // them, so migration keeps them as written and names the setting. assert_eq!( - folders, - &serde_json::json!([ - format!("{home_path}/work"), - format!("{home_path}/hop/../work"), - // Already names the environment's copy. - "~/.openclaw/wf", - // Leaves the state directory, so the relocation of copied - // state paths keeps it. - format!("{home_path}/.openclaw/../work"), - // Kept, so it still names a missing directory in the copy - // instead of being resolved to the copied `wf`. - "~/.openclaw/missing/../wf", - format!("{home_path}/.openclaw/../../shared"), - // OpenClaw does not trim, so this is not home-relative. - " ~/work", - ]), + copied["nodeHost"]["workerRuns"]["folders"], folders, "adopt={adopt}" ); - assert_eq!( - fs::canonicalize(folders[1].as_str().unwrap()).unwrap(), - fs::canonicalize(home.join("data/work")).unwrap() - ); let warnings = stderr(&imported); assert_eq!( warnings .lines() .filter(|line| line.starts_with( - "warning: env copied keeps the ~/ entries of nodeHost.workerRuns.folders[] whose .. leads into ~/.openclaw as written" + "warning: env copied keeps the ~/ entries of nodeHost.workerRuns.folders[] as written" )) .count(), 1, @@ -5581,24 +5558,16 @@ fn migrate_and_adopt_expand_worker_folders_without_resolving_parent_traversal() } } -#[cfg(unix)] #[test] -fn migrate_and_adopt_keep_worker_folders_resolved_through_an_imported_workspace() { +fn migrate_and_adopt_do_not_name_absolute_worker_folders() { for adopt in [false, true] { - let root = TestDir::new("migrate-home-relative-worker-workspace"); + let root = TestDir::new("migrate-home-relative-worker-folders-absolute"); let env = ocm_env(&root); let home = root.child("home"); let source = home.join(".openclaw"); - fs::create_dir_all(source.join("wf")).unwrap(); - fs::create_dir_all(home.join("external/deep")).unwrap(); - fs::create_dir_all(home.join("external/wf")).unwrap(); - // OpenClaw resolves `workspace/../wf` to `~/external/wf`. Migration - // imports the linked workspace, so the copied state no longer has - // that link and the same entry would name the copied `wf`. - std::os::unix::fs::symlink(home.join("external/deep"), source.join("workspace")).unwrap(); + fs::create_dir_all(&source).unwrap(); let config = serde_json::json!({ - "agents": {"defaults": {"workspace": "~/.openclaw/workspace"}}, - "nodeHost": {"workerRuns": {"folders": ["~/.openclaw/workspace/../wf"]}} + "nodeHost": {"workerRuns": {"folders": ["/srv/work", " ~/work"]}} }); support::write_text(&source.join("openclaw.json"), &config.to_string()); @@ -5608,20 +5577,8 @@ fn migrate_and_adopt_keep_worker_folders_resolved_through_an_imported_workspace( &migrate_or_adopt_args(adopt, "copied", &source), ); assert!(imported.status.success(), "{}", stderr(&imported)); - - let target = root.child("ocm-home/envs/copied/.openclaw"); - let copied: serde_json::Value = - serde_json::from_str(&fs::read_to_string(target.join("openclaw.json")).unwrap()) - .unwrap(); - assert_eq!( - copied["nodeHost"]["workerRuns"]["folders"], - serde_json::json!(["~/.openclaw/workspace/../wf"]), - "adopt={adopt}" - ); assert!( - stderr(&imported).lines().any(|line| line.starts_with( - "warning: env copied keeps the ~/ entries of nodeHost.workerRuns.folders[] whose .. leads into ~/.openclaw as written" - )), + !stderr(&imported).contains("nodeHost.workerRuns.folders"), "adopt={adopt}: {}", stderr(&imported) ); From c7d29905251beb7d5832d92a981b6ea71d79a36e Mon Sep 17 00:00:00 2001 From: Shakker Date: Mon, 2 Nov 2026 12:23:22 +0000 Subject: [PATCH 7/8] fix: keep placeholder paths the state relocation would move through a link A spelled-out placeholder value that leads into `~/.openclaw` through a symlink would be relocated into the env as if the placeholder were a directory name, while OpenClaw substitutes it before resolving `..`. Such values stay as written and are named in the placeholder warning. An escaped `$${VAR}` is no longer treated as a placeholder. --- src/store/openclaw_config.rs | 79 ++++++++++++++++++++++++++++++---- tests/migrate_command_tests.rs | 45 +++++++++++++++++++ 2 files changed, 115 insertions(+), 9 deletions(-) diff --git a/src/store/openclaw_config.rs b/src/store/openclaw_config.rs index 453046fa..6d168afd 100644 --- a/src/store/openclaw_config.rs +++ b/src/store/openclaw_config.rs @@ -237,7 +237,14 @@ pub(crate) fn rewrite_openclaw_config_for_migration( gateway_port: Option, sandbox_origin: Option<&str>, ) -> Result { - let kept = expand_migrated_home_relative_paths(target_paths, source_home)?; + let kept = expand_migrated_home_relative_paths( + target_paths, + &HomeExpansion { + home: source_home, + source_state_dir: source_state_root, + target_state_dir: &target_paths.state_dir, + }, + )?; rewrite_migrated_plugin_load_paths( target_paths, source_state_root, @@ -263,10 +270,10 @@ pub(crate) fn rewrite_openclaw_config_for_migration( /// although they may now resolve elsewhere. fn expand_migrated_home_relative_paths( target: &EnvPaths, - source_home: &Path, + expansion: &HomeExpansion<'_>, ) -> Result, String> { let mut kept = BTreeSet::new(); - if !source_home.is_absolute() { + if !expansion.home.is_absolute() { return Ok(kept); } let Some(effective) = load_effective_openclaw_config(&target.config_path)? else { @@ -281,7 +288,7 @@ fn expand_migrated_home_relative_paths( let mut rewritten = None; for mut scope in scopes { let mut candidate = value.clone(); - expand_home_relative_paths_in_scope(&mut candidate, &mut scope, source_home, &mut kept); + expand_home_relative_paths_in_scope(&mut candidate, &mut scope, expansion, &mut kept); if rewritten .as_ref() .is_some_and(|previous| previous != &candidate) @@ -300,15 +307,33 @@ fn expand_migrated_home_relative_paths( Ok(kept) } +/// The home `~` named in the plain install, and the state directory the +/// later relocation moves absolute values from and to. +struct HomeExpansion<'a> { + home: &'a Path, + source_state_dir: &'a Path, + target_state_dir: &'a Path, +} + +impl HomeExpansion<'_> { + /// Whether the later relocation of absolute state paths would move this + /// spelled-out value into the copied state directory. + fn relocates(&self, expanded: &str) -> bool { + let mut relocated = expanded.to_string(); + rewrite_env_root_string(&mut relocated, self.source_state_dir, self.target_state_dir) + } +} + /// Expands the home-relative path settings in a config fragment included at /// `scope`, and records the settings it keeps as written although they may /// now resolve elsewhere. `$include` directives keep their own relocation. fn expand_home_relative_paths_in_scope( value: &mut Value, scope: &mut Vec, - home: &Path, + expansion: &HomeExpansion<'_>, kept: &mut BTreeSet, ) -> bool { + let home = expansion.home; match value { Value::String(raw) => { let Some(rule) = home_relative_path_setting(scope) else { @@ -317,6 +342,15 @@ fn expand_home_relative_paths_in_scope( let expanded = match rule { HomePathRule::Lexical { trims } => { match expand_home_relative_path(raw, home, trims) { + // The later relocation of state paths follows links + // and would treat the placeholder as a directory name, + // which OpenClaw substitutes before resolving `..`. + Some(expanded) + if has_home_relative_placeholder(raw, trims) + && expansion.relocates(&expanded) => + { + Err(KeptHomeRelativeReason::Placeholder) + } Some(expanded) => Ok(expanded), // A placeholder under `~/.openclaw` can still lead outside it. None if has_home_relative_placeholder(raw, trims) => { @@ -359,7 +393,7 @@ fn expand_home_relative_paths_in_scope( let mut changed = false; for (index, nested) in values.iter_mut().enumerate() { scope.push(ConfigSegment::Index(index)); - changed |= expand_home_relative_paths_in_scope(nested, scope, home, kept); + changed |= expand_home_relative_paths_in_scope(nested, scope, expansion, kept); scope.pop(); } changed @@ -368,7 +402,7 @@ fn expand_home_relative_paths_in_scope( let mut changed = false; for (key, nested) in values.iter_mut().filter(|(key, _)| *key != "$include") { scope.push(ConfigSegment::Key(key.clone())); - changed |= expand_home_relative_paths_in_scope(nested, scope, home, kept); + changed |= expand_home_relative_paths_in_scope(nested, scope, expansion, kept); scope.pop(); } changed @@ -540,7 +574,25 @@ fn has_home_relative_placeholder(raw: &str, trims: bool) -> bool { } else { raw }; - is_home_relative(value) && value.contains("${") + is_home_relative(value) && first_placeholder(value).is_some() +} + +/// Where the first `${` that OpenClaw may substitute starts. `$${` is its +/// escape for the literal text `${`. +fn first_placeholder(value: &str) -> Option { + let mut search = 0; + while let Some(offset) = value[search..].find('$') { + let index = search + offset; + let rest = &value[index + 1..]; + if rest.starts_with("${") { + search = index + 3; + } else if rest.starts_with('{') { + return Some(index); + } else { + search = index + 1; + } + } + None } /// Splits a home-relative value before the separator that precedes its @@ -548,7 +600,7 @@ fn has_home_relative_placeholder(raw: &str, trims: bool) -> bool { /// resolves the path, so a placeholder's value can still add `..` or name /// any directory; only the literal directories before it are known here. fn split_before_placeholder(value: &str) -> (&str, &str) { - let Some(placeholder) = value.find("${") else { + let Some(placeholder) = first_placeholder(value) else { return (value, ""); }; // `~` is followed by a separator, so the split never falls before it. @@ -3153,6 +3205,15 @@ mod tests { Some("/\\${X}") ); assert_eq!(expand("~/.openclaw/${SUB}/x"), None); + // `$${` is OpenClaw's escape for a literal `${`. + assert_eq!(first_placeholder("a$${b}/${c}"), Some(7)); + assert_eq!(first_placeholder("~/dev/$${X}"), None); + assert!(!has_home_relative_placeholder("~/.openclaw/$${X}", true)); + assert!(has_home_relative_placeholder(" ~/.openclaw/${X}", true)); + assert_eq!( + expand("~/dev/$${X}/../p").as_deref(), + Some("/home/alice/dev/p") + ); assert_eq!( expand("~/.openclaw/../${SUB}").as_deref(), Some("/home/alice/${SUB}") diff --git a/tests/migrate_command_tests.rs b/tests/migrate_command_tests.rs index 0845501d..3a41161e 100644 --- a/tests/migrate_command_tests.rs +++ b/tests/migrate_command_tests.rs @@ -5733,6 +5733,51 @@ fn migrate_and_adopt_refuse_an_include_shared_by_a_path_list_and_a_trust_root() } } +#[cfg(unix)] +#[test] +fn migrate_and_adopt_keep_placeholder_paths_that_lead_into_the_state_dir_through_a_link() { + for adopt in [false, true] { + let root = TestDir::new("migrate-home-relative-placeholder-alias"); + let env = ocm_env(&root); + let home = root.child("home"); + let source = home.join(".openclaw"); + fs::create_dir_all(&source).unwrap(); + // With `SUB=../external`, OpenClaw resolves `~/alias/${SUB}` to + // `~/external`; relocating the spelled-out value through the link + // would name the environment's `external` instead. + std::os::unix::fs::symlink(&source, home.join("alias")).unwrap(); + let config = serde_json::json!({ + "skills": {"load": {"extraDirs": ["~/alias/${SUB}", "~/shared/${SUB}"]}} + }); + support::write_text(&source.join("openclaw.json"), &config.to_string()); + + let imported = run_ocm( + root.path(), + &env, + &migrate_or_adopt_args(adopt, "copied", &source), + ); + assert!(imported.status.success(), "{}", stderr(&imported)); + + let target = root.child("ocm-home/envs/copied/.openclaw"); + let copied: serde_json::Value = + serde_json::from_str(&fs::read_to_string(target.join("openclaw.json")).unwrap()) + .unwrap(); + let home_path = home.to_str().unwrap(); + assert_eq!( + copied["skills"]["load"]["extraDirs"], + serde_json::json!(["~/alias/${SUB}", format!("{home_path}/shared/${{SUB}}")]), + "adopt={adopt}" + ); + assert!( + stderr(&imported).lines().any(|line| line.starts_with( + "warning: env copied keeps the ~/ entries of skills.load.extraDirs[] that contain a ${VAR} placeholder" + )), + "adopt={adopt}: {}", + stderr(&imported) + ); + } +} + #[test] fn migrate_and_adopt_do_not_name_trust_roots_that_keep_their_meaning() { for adopt in [false, true] { From 4065c0d91a5971dbbc9f61cf0e7e6fef1c72c3a1 Mon Sep 17 00:00:00 2001 From: Shakker Date: Mon, 2 Nov 2026 12:29:49 +0000 Subject: [PATCH 8/8] fix: keep escaped placeholder paths the state relocation would move OpenClaw unescapes `$${X}` to the literal `${X}` before resolving a path, so a spelled-out value whose `$${X}` component is a link into `~/.openclaw` would be relocated to the wrong directory. The relocation guard now covers every value containing `${`, escaped or not. USAGE and CHANGELOG name placeholder values that reach `~/.openclaw` through a symlink. --- CHANGELOG.md | 2 +- docs/USAGE.md | 21 +++++++++++---------- src/store/openclaw_config.rs | 10 ++++------ tests/migrate_command_tests.rs | 10 ++++++++-- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 71483b58..0b295adb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ All notable changes to OCM are documented here. - Find the installed npm launcher by default when building and installing runtimes on Windows. - Remove the hidden staging file and SQLite snapshot copies when `ocm env export` is stopped by SIGINT, SIGTERM or SIGHUP, and exit with status 130. Signals ignored at startup, as under `nohup`, stay ignored. - Keep `~/...` config paths outside `~/.openclaw`, such as local plugin, extra skills, agent directory and session or cron store paths, pointing at the same location after `migrate` and `adopt import`, instead of letting them resolve under the new environment root. Other config text is left unchanged. -- Also keep `~/...` values in includes placed at a list position and in values containing a `${VAR}` placeholder outside `~/.openclaw` pointing at the same location after `migrate` and `adopt import`. Home-relative trust roots in `skills.load.allowSymlinkTargets` and `security.installPolicy.exec.trustedDirs`, `~/.openclaw/...` values containing a placeholder, and `nodeHost.workerRuns.folders` entries stay as written, and migration now prints a warning naming each such setting, because they may resolve differently in the new environment. +- Also keep `~/...` values in includes placed at a list position and in values containing a `${VAR}` placeholder that do not lead into `~/.openclaw` pointing at the same location after `migrate` and `adopt import`. Home-relative trust roots in `skills.load.allowSymlinkTargets` and `security.installPolicy.exec.trustedDirs`, placeholder values that lead into `~/.openclaw`, and `nodeHost.workerRuns.folders` entries stay as written, and migration now prints a warning naming each such setting, because they may resolve differently in the new environment. ## 0.3.8 - 2026-10-29 diff --git a/docs/USAGE.md b/docs/USAGE.md index f018a2c8..2fb3f6fb 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -1293,16 +1293,17 @@ rewritten like any other. In a value containing a `${VAR}` placeholder, only the directories before the first placeholder are resolved; OpenClaw substitutes the placeholder and resolves the rest, as it would for the same value spelled from your home, so a placeholder that itself leads into `~/.openclaw` names the -source's state directory. A placeholder value under `~/.openclaw` stays as -written, and migration prints a warning naming its setting, because its -placeholder may still lead outside `~/.openclaw`. `nodeHost.workerRuns.folders` -entries are not rewritten: OpenClaw resolves them to real paths, through -symlinks, and grants workers access to the result, and migration can change the -links and directories they resolve through (for example when it imports a linked -workspace), so no spelling is known to keep the same folder. Their `~/...` -entries now resolve under the environment root, and migration prints a warning -naming the setting. Replace them with absolute paths to keep the original -folders ([#972](https://github.com/openclaw/ocm/issues/972)). The trust roots +source's state directory. A placeholder value under `~/.openclaw`, or one that +reaches it through a symlink, stays as written, and migration prints a warning +naming its setting, because the placeholder may still lead elsewhere once +OpenClaw substitutes it. `nodeHost.workerRuns.folders` entries are not +rewritten: OpenClaw resolves them to real paths, through symlinks, and grants +workers access to the result, and migration can change the links and directories +they resolve through (for example when it imports a linked workspace), so no +spelling is known to keep the same folder. Their `~/...` entries now resolve +under the environment root, and migration prints a warning naming the setting. +Replace them with absolute paths to keep the original folders +([#972](https://github.com/openclaw/ocm/issues/972)). The trust roots `skills.load.allowSymlinkTargets` and `security.installPolicy.exec.trustedDirs` are not rewritten, because that would change what the new environment trusts: their `~/...` entries outside `~/.openclaw` now resolve under the environment diff --git a/src/store/openclaw_config.rs b/src/store/openclaw_config.rs index 6d168afd..f662ceef 100644 --- a/src/store/openclaw_config.rs +++ b/src/store/openclaw_config.rs @@ -343,12 +343,10 @@ fn expand_home_relative_paths_in_scope( HomePathRule::Lexical { trims } => { match expand_home_relative_path(raw, home, trims) { // The later relocation of state paths follows links - // and would treat the placeholder as a directory name, - // which OpenClaw substitutes before resolving `..`. - Some(expanded) - if has_home_relative_placeholder(raw, trims) - && expansion.relocates(&expanded) => - { + // and would read a placeholder, or an escaped `$${`, + // as directory names, which OpenClaw substitutes or + // unescapes before it resolves the path. + Some(expanded) if raw.contains("${") && expansion.relocates(&expanded) => { Err(KeptHomeRelativeReason::Placeholder) } Some(expanded) => Ok(expanded), diff --git a/tests/migrate_command_tests.rs b/tests/migrate_command_tests.rs index 3a41161e..402bb5bc 100644 --- a/tests/migrate_command_tests.rs +++ b/tests/migrate_command_tests.rs @@ -5746,8 +5746,10 @@ fn migrate_and_adopt_keep_placeholder_paths_that_lead_into_the_state_dir_through // `~/external`; relocating the spelled-out value through the link // would name the environment's `external` instead. std::os::unix::fs::symlink(&source, home.join("alias")).unwrap(); + // OpenClaw reads `$${X}` as the directory `${X}`, not this link. + std::os::unix::fs::symlink(&source, home.join("$${X}")).unwrap(); let config = serde_json::json!({ - "skills": {"load": {"extraDirs": ["~/alias/${SUB}", "~/shared/${SUB}"]}} + "skills": {"load": {"extraDirs": ["~/alias/${SUB}", "~/shared/${SUB}", "~/$${X}/skills"]}} }); support::write_text(&source.join("openclaw.json"), &config.to_string()); @@ -5765,7 +5767,11 @@ fn migrate_and_adopt_keep_placeholder_paths_that_lead_into_the_state_dir_through let home_path = home.to_str().unwrap(); assert_eq!( copied["skills"]["load"]["extraDirs"], - serde_json::json!(["~/alias/${SUB}", format!("{home_path}/shared/${{SUB}}")]), + serde_json::json!([ + "~/alias/${SUB}", + format!("{home_path}/shared/${{SUB}}"), + "~/$${X}/skills" + ]), "adopt={adopt}" ); assert!(