diff --git a/CHANGELOG.md b/CHANGELOG.md index 02fb4781..b953d628 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ All notable changes to OCM are documented here. - 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 the Gateway running when `ocm service restart` is run again within seconds of a previous restart. The daemon no longer counts a restart handoff that `ocm service restart` requested toward its clean-exit restart loop guard, which left the Gateway stopped while the command waited 30 seconds and exited 0. A Gateway that keeps exiting with a handoff on its own is still stopped. - 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 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. - Name the environments whose Gateway port families hold a retained dev UI port when `ocm dev` refuses it, and offer `--no-ui`, instead of asking the user to release a port that no command can release. A port bound by a process keeps the existing "occupied or reserved" wording. ## 0.3.8 - 2026-10-29 diff --git a/docs/USAGE.md b/docs/USAGE.md index a5270d84..d69c0bb7 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -1288,8 +1288,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 @@ -1301,18 +1301,34 @@ 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; -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. +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`, 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 +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/cli/migrate.rs b/src/cli/migrate.rs index bf81bc3b..6f7b62f2 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_settings( + &result.summary.name, + &result.kept_home_relative_settings, + ); 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_settings( + &result.summary.name, + &result.kept_home_relative_settings, + ); let summary = result.summary; if json_flag { @@ -243,6 +251,27 @@ impl Cli { } } + 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 ~/ 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::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" + ), + }); + } + } + 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..f78e4d45 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_settings: Vec, } #[derive(Clone, Debug)] @@ -353,6 +354,7 @@ struct CompletedMigrationImport { cleared_sandbox_origin: bool, sandbox_port: Option, external_plugin_ids: Vec, + kept_home_relative_settings: 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_settings: completed.kept_home_relative_settings, }) } @@ -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_settings: config_rewrite.kept_home_relative_settings, }); }; @@ -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_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 45fafd22..f662ceef 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,32 @@ pub(crate) struct OpenClawConfigAudit { pub(crate) struct OpenClawConfigRewriteOutcome { pub cleared_sandbox_origin: bool, pub sandbox_port: Option, + /// 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, + /// 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( @@ -211,18 +237,27 @@ 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 = 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, 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_settings = kept.into_iter().collect(); + Ok(outcome) } /// A plain install expands `~` against its home, but the environment runs @@ -231,15 +266,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 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> { - if !source_home.is_absolute() { - return Ok(()); + expansion: &HomeExpansion<'_>, +) -> Result, String> { + let mut kept = BTreeSet::new(); + if !expansion.home.is_absolute() { + return Ok(kept); } let Some(effective) = load_effective_openclaw_config(&target.config_path)? else { - return Ok(()); + return Ok(kept); }; let mut scopes = effective.include_scopes; scopes.insert(target.config_path.clone(), BTreeSet::from([Vec::new()])); @@ -248,13 +286,9 @@ 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, expansion, &mut kept); if rewritten .as_ref() .is_some_and(|previous| previous != &candidate) @@ -270,39 +304,94 @@ fn expand_migrated_home_relative_paths( write_copied_config_value(&path, &target.state_dir, &rewritten)?; } } - Ok(()) + 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, } -/// 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, +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`. `$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, + expansion: &HomeExpansion<'_>, + kept: &mut BTreeSet, ) -> bool { + let home = expansion.home; match value { Value::String(raw) => { - let Some(trims) = home_relative_path_setting(scope) else { + let Some(rule) = home_relative_path_setting(scope) else { return false; }; - let Some(expanded) = expand_home_relative_path(raw, home, trims) else { - return false; + 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 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), + // A placeholder under `~/.openclaw` can still lead outside it. + None if has_home_relative_placeholder(raw, trims) => { + Err(KeptHomeRelativeReason::Placeholder) + } + None => return false, + } + } + 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() + && !has_home_relative_placeholder(raw, true) + { + return false; + } + Err(KeptHomeRelativeReason::TrustRoot) + } }; - *raw = expanded; - true + match expanded { + Ok(expanded) => { + *raw = expanded; + true + } + Err(reason) => { + kept.insert(KeptHomeRelativeSetting { + setting: config_setting_name(scope), + reason, + }); + false + } + } } 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, expansion, kept); scope.pop(); } changed @@ -311,7 +400,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); + changed |= expand_home_relative_paths_in_scope(nested, scope, expansion, kept); scope.pop(); } changed @@ -320,53 +409,114 @@ 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 }, + /// 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, +} + +/// 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` /// 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, to decide which folders + // workers may use. + ( + &["nodeHost", "workerRuns", "folders", "[]"], + HomePathRule::WorkerFolder, + ), + // src/skills/loading/symlink-targets.ts and src/security/install-policy.ts + ( + &["skills", "load", "allowSymlinkTargets", "[]"], + HomePathRule::TrustRoot, + ), + ( + &["security", "installPolicy", "exec", "trustedDirs", "[]"], + HomePathRule::TrustRoot, + ), ]; -/// 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 +524,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 +536,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 +560,81 @@ fn is_ecmascript_whitespace(c: char) -> bool { ) } -fn expand_home_relative_path(raw: &str, home: &Path, trims: bool) -> Option { +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 }; - if value != "~" && !value.starts_with("~/") && !value.starts_with("~\\") { - return None; + 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; + } } - // 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("${") { + None +} + +/// 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) = first_placeholder(value) 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 !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.strip_prefix(std::path::is_separator).unwrap_or(rest); + } + Some(format!("{expanded}{rest}")) } /// Moves a Gateway port that a copied `$include` supplies to `gateway_port`, @@ -456,7 +657,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 +712,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 +775,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 +803,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 +816,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 +1303,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 +1319,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 +3173,49 @@ 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}")); + // 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); + // `$${` 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}") + ); // 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 +3230,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 +3253,28 @@ 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::WorkerFolder) + ); + assert_eq!( + setting(&["skills", "load", "allowSymlinkTargets", "[]"]), + Some(HomePathRule::TrustRoot) + ); + assert_eq!( + setting(&["security", "installPolicy", "exec", "trustedDirs", "[]"]), + Some(HomePathRule::TrustRoot) + ); for text in [ &["messages", "responsePrefix"][..], &["messages", "usageTemplate"], @@ -3034,23 +3291,64 @@ 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:?}"); } } + #[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 }) + ); + assert_eq!( + config_setting_name(scopes.first().unwrap()), + "hooks.items[].paths[]" + ); + } + 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..415a56a2 100644 --- a/src/store/openclaw_workspaces.rs +++ b/src/store/openclaw_workspaces.rs @@ -37,7 +37,31 @@ 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, + // 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) + } + } + } } #[derive(Clone, Debug, PartialEq, Eq)] @@ -1542,16 +1566,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 +1583,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 +1665,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 +1693,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 +1718,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 +1740,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 +1758,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 +1795,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 +1820,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 +1845,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..402bb5bc 100644 --- a/tests/migrate_command_tests.rs +++ b/tests/migrate_command_tests.rs @@ -5187,12 +5187,12 @@ 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. + // Kept as written and named in a warning; see the worker-folder test. assert_eq!( copied["nodeHost"]["workerRuns"]["folders"], serde_json::json!(["~/work"]) ); + // Expanded against the OS home, which the environment keeps. assert_eq!( copied["memory"]["search"]["extraPaths"], serde_json::json!(["~/notes"]) @@ -5380,7 +5380,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 +5388,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 +5464,350 @@ 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}"); + // 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 ~/ entries of skills.load.extraDirs[] that contain a ${VAR} placeholder and point into ~/.openclaw" + )) + .count(), + 1, + "adopt={adopt}: {warnings}" + ); + assert!(!warnings.contains("logging.file"), "{warnings}"); + } +} + +#[cfg(unix)] +#[test] +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); + 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("work")).unwrap(); + std::os::unix::fs::symlink(home.join("data/deep"), home.join("hop")).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": folders}} + }); + 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(); + // OpenClaw resolves these to real paths and grants workers access to + // them, so migration keeps them as written and names the setting. + assert_eq!( + copied["nodeHost"]["workerRuns"]["folders"], folders, + "adopt={adopt}" + ); + let warnings = stderr(&imported); + assert_eq!( + warnings + .lines() + .filter(|line| line.starts_with( + "warning: env copied keeps the ~/ entries of nodeHost.workerRuns.folders[] as written" + )) + .count(), + 1, + "adopt={adopt}: {warnings}" + ); + } +} + +#[test] +fn migrate_and_adopt_do_not_name_absolute_worker_folders() { + for adopt in [false, true] { + 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).unwrap(); + let config = serde_json::json!({ + "nodeHost": {"workerRuns": {"folders": ["/srv/work", " ~/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)); + assert!( + !stderr(&imported).contains("nodeHost.workerRuns.folders"), + "adopt={adopt}: {}", + stderr(&imported) + ); + } +} + +#[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(&format!( + "warning: env copied keeps the ~/ entries of trust root {setting}[] as written" + ))) + .count(), + 1, + "adopt={adopt}: {warnings}" + ); + } + } +} + +#[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""# + ); + } +} + +#[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(); + // 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}", "~/$${X}/skills"]}} + }); + 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}}"), + "~/$${X}/skills" + ]), + "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] { + 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) ); } }