feat: add extensible deployment profiles (IRONCLAW_PROFILE) - #2203
Conversation
Add a profile system that lets users select a deployment shape with a single env var. Profiles are partial Settings TOML files merged onto defaults before config.toml and DB overlays. Built-in profiles: local, local-sandbox, server, server-multitenant. Users can create custom profiles in ~/.ironclaw/profiles/. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces deployment profiles, which are TOML-based configuration presets (e.g., local, server) that can be selected via the IRONCLAW_PROFILE environment variable. These profiles are merged into the application settings, allowing for easier environment-specific configurations. Feedback identifies a high-severity path traversal vulnerability in the profile loading logic and suggests normalizing profile names to lowercase for consistency between user-defined and built-in profiles.
| fn load_profile(name: &str) -> Result<Settings, ConfigError> { | ||
| // 1. Check user-defined profiles directory. | ||
| let user_path = user_profiles_dir().join(format!("{name}.toml")); | ||
| if user_path.is_file() { | ||
| return Settings::load_toml(&user_path) | ||
| .map_err(ConfigError::ParseError)? | ||
| .ok_or(ConfigError::InvalidValue { | ||
| key: "IRONCLAW_PROFILE".to_string(), | ||
| message: format!( | ||
| "profile file exists but could not be loaded: {}", | ||
| user_path.display() | ||
| ), | ||
| }); | ||
| } | ||
|
|
||
| // 2. Check built-in profiles. | ||
| let normalized = name.to_ascii_lowercase(); | ||
| for &(builtin_name, toml_content) in BUILTIN_PROFILES { | ||
| if builtin_name == normalized { |
There was a problem hiding this comment.
The name parameter, derived from the IRONCLAW_PROFILE environment variable, is used to construct a file path without sanitization, which could allow for path traversal (e.g., IRONCLAW_PROFILE=../../etc/passwd).
Additionally, there is an inconsistency in case-sensitivity: the filesystem check is case-sensitive on Linux, while the built-in check is case-insensitive. Normalizing the name to lowercase at the start of the function resolves both the inconsistency and ensures that user overrides are correctly identified regardless of the environment variable's casing.
fn load_profile(name: &str) -> Result<Settings, ConfigError> {
let name = name.to_ascii_lowercase();
// Prevent path traversal
if name.contains('/') || name.contains('\\') || name.contains("..") {
return Err(ConfigError::InvalidValue {
key: "IRONCLAW_PROFILE".to_string(),
message: format!("invalid profile name: {name}"),
});
}
// 1. Check user-defined profiles directory.
let user_path = user_profiles_dir().join(format!("{name}.toml"));
if user_path.is_file() {
return Settings::load_toml(&user_path)
.map_err(ConfigError::ParseError)?
.ok_or(ConfigError::InvalidValue {
key: "IRONCLAW_PROFILE".to_string(),
message: format!(
"profile file exists but could not be loaded: {}",
user_path.display()
),
});
}
// 2. Check built-in profiles.
for &(builtin_name, toml_content) in BUILTIN_PROFILES {
if builtin_name == name {References
- This rule emphasizes validating user-provided names at the public API boundary to prevent path traversal when constructing file paths, which applies to the
IRONCLAW_PROFILEenvironment variable. - This rule requires validation of path traversal characters in user-provided names at the public API boundary to prevent security vulnerabilities, directly addressing the issue identified.
| if path.extension().is_some_and(|ext| ext == "toml") | ||
| && let Some(stem) = path.file_stem().and_then(|s| s.to_str()) | ||
| { | ||
| let name = stem.to_string(); |
There was a problem hiding this comment.
Normalize the profile name to lowercase here to ensure consistency with the built-in profile names and the loading logic. This ensures that a file named Server.toml is correctly identified as an override for the built-in server profile.
| let name = stem.to_string(); | |
| let name = stem.to_ascii_lowercase(); |
There was a problem hiding this comment.
Pull request overview
Adds a deployment “profile” system selected via IRONCLAW_PROFILE to pre-seed Settings from built-in or user-provided TOML presets before existing config overlays, reducing per-deployment env var sprawl.
Changes:
- Introduces
src/config/profile.rsto load/apply profiles from~/.ironclaw/profiles/<name>.toml(preferred) or embedded built-ins. - Wires profile application into config construction paths so profiles layer onto
Settings::default()ahead of TOML + DB overlays. - Adds four built-in profile TOML files (
local,local-sandbox,server,server-multitenant) and a newchannels.cli_modesetting to support TUI defaults.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/settings.rs | Adds channels.cli_mode to settings + default implementation. |
| src/config/profile.rs | Implements profile discovery/loading/apply + unit tests. |
| src/config/mod.rs | Applies profile during config load/resolve paths. |
| src/config/channels.rs | Resolves CLI_MODE via settings/DB-first optional string to support profiles/TOML. |
| profiles/local.toml | Built-in “local” preset (libsql, no gateway/background features, TUI). |
| profiles/local-sandbox.toml | Built-in “local-sandbox” preset (local + sandbox). |
| profiles/server.toml | Built-in “server” preset (postgres, sandbox, heartbeat, hygiene). |
| profiles/server-multitenant.toml | Built-in “server-multitenant” preset (server + higher parallelism). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Load a profile by name, checking user directory first, then built-ins. | ||
| fn load_profile(name: &str) -> Result<Settings, ConfigError> { | ||
| // 1. Check user-defined profiles directory. | ||
| let user_path = user_profiles_dir().join(format!("{name}.toml")); | ||
| if user_path.is_file() { |
There was a problem hiding this comment.
IRONCLAW_PROFILE is interpolated directly into a filename (profiles_dir.join(format!("{name}.toml"))). This allows path traversal and absolute-path reads (e.g. IRONCLAW_PROFILE=../../etc/passwd or /tmp/foo) and can escape the intended ~/.ironclaw/profiles/ directory. Validate/sanitize the profile name before building a path (reject '/', '\', '..', NUL, and absolute paths; optionally restrict to [a-z0-9_-] and/or lowercase the name consistently).
| let mut settings = Settings::load(); | ||
| profile::apply_profile(&mut settings)?; | ||
| Config::apply_toml_overlay(&mut settings, toml_path)?; |
There was a problem hiding this comment.
load_bootstrap_settings() loads legacy settings.json first and then applies the deployment profile. This makes profile values override existing on-disk settings, which contradicts the stated goal that profiles are a preset base and that user overrides still win. Consider starting from Settings::default(), applying the profile, then merging settings.json (and then TOML) so user-configured values keep priority over the selected profile.
| #[test] | ||
| fn user_profile_overrides_builtin() { | ||
| let _guard = lock_env(); | ||
|
|
||
| // Create a temporary user profile. | ||
| let tmp = tempfile::TempDir::new().unwrap(); | ||
| let profile_path = tmp.path().join("local.toml"); | ||
| std::fs::write( | ||
| &profile_path, | ||
| "database_backend = \"postgres\"\n\n[heartbeat]\nenabled = true\n", | ||
| ) | ||
| .unwrap(); | ||
|
|
||
| // Load it directly via load_profile logic (simulating user dir). | ||
| let loaded: Settings = Settings::load_toml(&profile_path).unwrap().unwrap(); | ||
| assert_eq!(loaded.database_backend.as_deref(), Some("postgres")); | ||
| assert!(loaded.heartbeat.enabled); | ||
| } |
There was a problem hiding this comment.
The user_profile_overrides_builtin test doesn't exercise the profile lookup/override logic (it just calls Settings::load_toml on a temp file). This can pass even if apply_profile()/load_profile() never consults the user profiles directory or the built-in-vs-user precedence is wrong. It would be more robust to set up a temp base dir with profiles/local.toml, set IRONCLAW_PROFILE=local, call apply_profile(), and assert the user file overrides the embedded built-in profile.
…erride order, formatting - Sanitize IRONCLAW_PROFILE to reject path traversal attempts (/, \, ..) - Normalize profile name to lowercase for both user-path and built-in lookups - Fix load_bootstrap_settings to use Settings::default() matching from_env() pattern - Collapse .unwrap_or_default() onto one line to satisfy rustfmt - Improve user_profile_overrides_builtin test to exercise actual merge logic - Add path_traversal_rejected test with 5 malicious name patterns Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
henrypark133
left a comment
There was a problem hiding this comment.
Review: Extensible deployment profiles via IRONCLAW_PROFILE
Clean, well-designed feature. The profile system slots neatly into the existing config layering (defaults → profile → TOML → DB → env) and the include_str! embedding keeps profiles discoverable in-repo while shipping them in the binary.
Positives:
- Path traversal protection with rejection of
/,\,.., and leading.— addressed proactively in commit 2 - Case-insensitive lookup with early lowercase normalization
- Good error messages that list available profiles and suggest the user-defined path
- Comprehensive test coverage (7 tests including traversal, case insensitivity, merge layering)
- Profile TOML files are well-documented with prerequisites and usage instructions
Minor notes:
- The
unsafeenv var manipulation in tests is fine given thelock_env()guard — just noting it uses the Rust 1.66+ unsafe requirement correctly list_profiles()won't be exercised by a CLI command yet — presumably that comes with a follow-upironclaw profile listsubcommand
LGTM.
Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com
* feat(cli): add `ironclaw profile list` subcommand Wire the existing `list_profiles()` function from the deployment profile system (#2203) into a new CLI subcommand so users can discover available profiles, see which one is active, and read descriptions. Closes #2271 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(cli): address review feedback on profile list command - Fix import ordering to satisfy rustfmt (BUILTIN_PROFILES, ProfileInfo, list_profiles) - Log warning instead of silently ignoring errors when reading user profile files - Replace manual serde_json::Value construction with typed ProfileEntry struct Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
) * feat: add extensible deployment profiles (IRONCLAW_PROFILE) Add a profile system that lets users select a deployment shape with a single env var. Profiles are partial Settings TOML files merged onto defaults before config.toml and DB overlays. Built-in profiles: local, local-sandbox, server, server-multitenant. Users can create custom profiles in ~/.ironclaw/profiles/. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address review comments — path traversal, case normalization, override order, formatting - Sanitize IRONCLAW_PROFILE to reject path traversal attempts (/, \, ..) - Normalize profile name to lowercase for both user-path and built-in lookups - Fix load_bootstrap_settings to use Settings::default() matching from_env() pattern - Collapse .unwrap_or_default() onto one line to satisfy rustfmt - Improve user_profile_overrides_builtin test to exercise actual merge logic - Add path_traversal_rejected test with 5 malicious name patterns Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* feat(cli): add `ironclaw profile list` subcommand Wire the existing `list_profiles()` function from the deployment profile system (nearai#2203) into a new CLI subcommand so users can discover available profiles, see which one is active, and read descriptions. Closes nearai#2271 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(cli): address review feedback on profile list command - Fix import ordering to satisfy rustfmt (BUILTIN_PROFILES, ProfileInfo, list_profiles) - Log warning instead of silently ignoring errors when reading user profile files - Replace manual serde_json::Value construction with typed ProfileEntry struct Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
IRONCLAW_PROFILE=serverpre-seeds sensible defaults, replacing dozens of env vars with onelocal,local-sandbox,server,server-multitenant~/.ironclaw/profiles/Settings::default()before config.toml and DB overlays, so individual settings still overrideTest plan
cargo check --all-featurescompilescargo clippy— zero warnings🤖 Generated with Claude Code