Skip to content

Commit ca912e1

Browse files
amiable-devclaude
andcommitted
fix(gui): profile creation bug and UX improvements (v4.10.2)
## Bug Fix (#52) - Profile creation now works when frontend sends empty config_path - Backend auto-generates config_path using sanitized profile ID - Creates default config from template or copies main config - SECURITY: Validates path BEFORE file I/O (prevents TOCTOU) - SECURITY: Uses create_new(true) for atomic file creation (Council review fix) - 3 TDD tests verify auto-generation behavior ## UX Improvement (#53) - App Profiles moved from modal to main sidebar navigation - Created dedicated ProfilesView.svelte component - DevicesView now focuses on device/template management - Added PROFILES section to navigation store ## Files Changed - profile_manager.rs: Auto-generate config_path with atomic creation + tests - resources/default_profile.toml: Default profile template (NEW) - ProfilesView.svelte: Dedicated profile view (NEW) - App.svelte: Added PROFILES routing - Sidebar.svelte: Added App Profiles nav item - navigation.js: Added PROFILES section - DevicesView.svelte: Removed ProfileManager modal code - Cargo.toml: Bumped version to 4.10.2 - CHANGELOG.md: v4.10.2 release notes Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent e98fa3d commit ca912e1

File tree

10 files changed

+610
-267
lines changed

10 files changed

+610
-267
lines changed

CHANGELOG.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
- Action macros and scripting
1313
- Cloud sync (optional)
1414

15+
## [4.10.2] - 2026-01-30
16+
17+
### Fixed
18+
- **Profile Creation Bug** (#52): Fixed profile creation failing when frontend sends empty config_path
19+
- Backend now auto-generates config_path using sanitized profile ID
20+
- Creates default config file from template or copies main config
21+
- Added `create_default_profile_config()` helper method
22+
- 3 TDD tests verify auto-generation behavior
23+
24+
### Changed
25+
- **App Profiles Moved to Sidebar** (#53): Improved UX for per-app profile management
26+
- App Profiles now accessible directly from main sidebar navigation
27+
- Created dedicated `ProfilesView.svelte` component
28+
- Removed ProfileManager modal from DevicesView
29+
- DevicesView now focused solely on device/template management
30+
31+
### Added
32+
- **Default Profile Template**: New `resources/default_profile.toml` for auto-generated profiles
33+
- Contains device settings, modes, and example mappings
34+
- Used when no main config exists to copy from
35+
1536
## [4.10.1] - 2026-01-30
1637

1738
### Security

Cargo.lock

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ exclude = [
1313
resolver = "2"
1414

1515
[workspace.package]
16-
version = "4.10.1"
16+
version = "4.10.2"
1717
edition = "2024"
1818
authors = ["Conductor Contributors"]
1919
license = "MIT"
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# Default profile configuration
2+
# Created automatically for new profiles
3+
# Customize this file to define your MIDI mappings
4+
5+
[device]
6+
name = "Default Profile"
7+
auto_connect = true
8+
9+
[advanced_settings]
10+
chord_timeout_ms = 50
11+
double_tap_timeout_ms = 300
12+
hold_threshold_ms = 2000
13+
14+
[[modes]]
15+
name = "Default"
16+
color = "blue"
17+
18+
# Example mapping (uncomment and modify):
19+
# [[modes.mappings]]
20+
# trigger = { type = "Note", note = 36 }
21+
# action = { type = "Keystroke", keys = ["cmd", "space"] }

conductor-gui/src-tauri/src/profile_manager.rs

Lines changed: 211 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,11 @@ impl ProfileManager {
243243
/// # Security (ADR-006)
244244
/// - Validates profile_id doesn't contain path traversal characters
245245
/// - Validates config_path is within profiles directory
246-
pub async fn register_profile(&self, profile: AppProfile) -> Result<(), String> {
246+
///
247+
/// # Auto-generation (v4.10.2 - GitHub Issue #52)
248+
/// If config_path is empty, auto-generates a path based on profile_id
249+
/// and creates a default config file from the template.
250+
pub async fn register_profile(&self, mut profile: AppProfile) -> Result<(), String> {
247251
// SECURITY: Validate profile_id before registration (ADR-006)
248252
validate_profile_id(&profile.id).map_err(|e| format!("Invalid profile ID: {}", e))?;
249253

@@ -252,12 +256,39 @@ impl ProfileManager {
252256
validate_bundle_id(bundle_id).map_err(|e| format!("Invalid bundle ID: {}", e))?;
253257
}
254258

255-
// SECURITY: Validate config_path before registration (ADR-006)
256-
// Use async version to avoid blocking the Tokio runtime
259+
// AUTO-GENERATE config_path if empty (v4.10.2 - GitHub Issue #52)
260+
// This allows the frontend to create profiles without specifying a path
261+
// SECURITY: Set path first, validate BEFORE any file I/O to prevent TOCTOU
262+
let needs_default_config = if profile.config_path.as_os_str().is_empty() {
263+
let sanitized_id = sanitize_profile_id(&profile.id);
264+
let config_filename = format!("{}.toml", sanitized_id);
265+
let config_path = self.profiles_dir.join(&config_filename);
266+
267+
profile.config_path = config_path;
268+
tracing::info!(
269+
"Auto-generated config_path for profile '{}': {}",
270+
profile.id,
271+
profile.config_path.display()
272+
);
273+
true // Mark that we need to create default config after validation
274+
} else {
275+
false
276+
};
277+
278+
// SECURITY: Validate config_path BEFORE any file operations (ADR-006)
279+
// This prevents TOCTOU vulnerabilities by ensuring path is safe before writes
257280
validate_config_path_async(&profile.config_path, &self.profiles_dir)
258281
.await
259282
.map_err(|e| format!("Invalid config path: {}", e))?;
260283

284+
// Create default config file AFTER validation, only if auto-generated
285+
// SECURITY: Use atomic creation (try_create_new) instead of exists() + write
286+
// to prevent TOCTOU race condition where attacker could create symlink between check and write
287+
if needs_default_config {
288+
self.try_create_default_profile_config(&profile.config_path)
289+
.await?;
290+
}
291+
261292
let profile_id = profile.id.clone();
262293

263294
// Add to profiles map
@@ -275,6 +306,65 @@ impl ProfileManager {
275306
Ok(())
276307
}
277308

309+
/// Atomically create a default profile config file from template (v4.10.2 - GitHub Issue #52)
310+
///
311+
/// SECURITY: Uses create_new(true) for atomic file creation to prevent TOCTOU.
312+
/// If file already exists, returns Ok (idempotent).
313+
///
314+
/// Strategy:
315+
/// 1. Try to copy from main config if it exists (preserves user's settings)
316+
/// 2. Fall back to embedded default template
317+
async fn try_create_default_profile_config(&self, path: &Path) -> Result<(), String> {
318+
use tokio::io::AsyncWriteExt;
319+
320+
// Determine content to write
321+
let config_dir = dirs::config_dir().ok_or("Failed to get config directory")?;
322+
let main_config = config_dir.join("midimon").join("config.toml");
323+
324+
let content: String = if main_config.exists() {
325+
tokio::fs::read_to_string(&main_config)
326+
.await
327+
.map_err(|e| format!("Failed to read main config: {}", e))?
328+
} else {
329+
include_str!("../resources/default_profile.toml").to_string()
330+
};
331+
332+
// SECURITY: Atomic file creation with create_new(true) - fails if file exists
333+
// This prevents TOCTOU race where attacker creates symlink between check and write
334+
let file_result = tokio::fs::OpenOptions::new()
335+
.write(true)
336+
.create_new(true) // Atomic: fails if file already exists
337+
.open(path)
338+
.await;
339+
340+
match file_result {
341+
Ok(mut file) => {
342+
file.write_all(content.as_bytes())
343+
.await
344+
.map_err(|e| format!("Failed to write profile config: {}", e))?;
345+
tracing::info!(
346+
"Created profile config{}: {}",
347+
if main_config.exists() {
348+
" from main config"
349+
} else {
350+
" from default template"
351+
},
352+
path.display()
353+
);
354+
Ok(())
355+
}
356+
Err(e) if e.kind() == std::io::ErrorKind::AlreadyExists => {
357+
// File already exists - this is OK, idempotent behavior
358+
tracing::debug!(
359+
"Profile config already exists (skipped creation): {}",
360+
path.display()
361+
);
362+
Ok(())
363+
}
364+
Err(e) => Err(format!("Failed to create profile config: {}", e)),
365+
}
366+
}
367+
278368
/// Get profile by ID
279369
pub async fn get_profile(&self, profile_id: &str) -> Option<AppProfile> {
280370
self.profiles.read().await.get(profile_id).cloned()
@@ -1527,4 +1617,122 @@ mod tests {
15271617
);
15281618
}
15291619
}
1620+
1621+
// =========================================================================
1622+
// Auto-generate config_path Tests (v4.10.2 - GitHub Issue #52)
1623+
// =========================================================================
1624+
1625+
#[tokio::test]
1626+
async fn test_register_profile_auto_generates_config_path() {
1627+
// Test that profiles with empty config_path get a generated path
1628+
let temp_dir = TempDir::new().unwrap();
1629+
let manager = ProfileManager::with_directory(temp_dir.path().to_path_buf()).unwrap();
1630+
1631+
// Profile with empty config_path (simulates frontend creating new profile)
1632+
let profile = AppProfile {
1633+
id: "test-profile".to_string(),
1634+
name: "Test Profile".to_string(),
1635+
bundle_ids: vec!["com.example.app".to_string()],
1636+
config_path: PathBuf::new(), // Empty!
1637+
last_modified: None,
1638+
is_default: false,
1639+
};
1640+
1641+
// Registration should succeed
1642+
let result = manager.register_profile(profile).await;
1643+
assert!(result.is_ok(), "Registration should succeed with empty config_path: {:?}", result);
1644+
1645+
// Verify config file was created
1646+
let expected_path = temp_dir.path().join("test-profile.toml");
1647+
assert!(
1648+
expected_path.exists(),
1649+
"Config file should be created at: {}",
1650+
expected_path.display()
1651+
);
1652+
1653+
// Verify profile has correct config_path set
1654+
let stored = manager.get_profile("test-profile").await;
1655+
assert!(stored.is_some(), "Profile should be retrievable");
1656+
let stored = stored.unwrap();
1657+
// Canonicalize both paths to handle macOS /var -> /private/var symlink
1658+
let stored_canonical = stored.config_path.canonicalize().unwrap();
1659+
let expected_canonical = expected_path.canonicalize().unwrap();
1660+
assert_eq!(
1661+
stored_canonical, expected_canonical,
1662+
"Profile should have auto-generated config_path"
1663+
);
1664+
1665+
// Verify the config file has valid content
1666+
let content = tokio::fs::read_to_string(&expected_path).await.unwrap();
1667+
assert!(
1668+
content.contains("[device]") || content.contains("name"),
1669+
"Config file should have valid TOML content: {}",
1670+
content
1671+
);
1672+
}
1673+
1674+
#[tokio::test]
1675+
async fn test_register_profile_preserves_existing_config_path() {
1676+
// Test that profiles with explicit config_path retain it
1677+
let temp_dir = TempDir::new().unwrap();
1678+
let manager = ProfileManager::with_directory(temp_dir.path().to_path_buf()).unwrap();
1679+
1680+
// Create existing config file
1681+
let config_path = temp_dir.path().join("existing.toml");
1682+
tokio::fs::write(&config_path, "# Existing config\n[device]\nname = \"Existing\"")
1683+
.await
1684+
.unwrap();
1685+
1686+
let profile = AppProfile {
1687+
id: "test-profile".to_string(),
1688+
name: "Test Profile".to_string(),
1689+
bundle_ids: vec![],
1690+
config_path: config_path.clone(),
1691+
last_modified: None,
1692+
is_default: false,
1693+
};
1694+
1695+
manager.register_profile(profile).await.unwrap();
1696+
1697+
// Verify original config_path is preserved
1698+
let stored = manager.get_profile("test-profile").await.unwrap();
1699+
assert_eq!(
1700+
stored.config_path, config_path,
1701+
"Should preserve explicit config_path"
1702+
);
1703+
1704+
// Verify original content wasn't overwritten
1705+
let content = tokio::fs::read_to_string(&config_path).await.unwrap();
1706+
assert!(
1707+
content.contains("Existing"),
1708+
"Original config content should be preserved"
1709+
);
1710+
}
1711+
1712+
#[tokio::test]
1713+
async fn test_register_profile_auto_generate_uses_sanitized_id() {
1714+
// Test that auto-generated paths use sanitized profile IDs
1715+
let temp_dir = TempDir::new().unwrap();
1716+
let manager = ProfileManager::with_directory(temp_dir.path().to_path_buf()).unwrap();
1717+
1718+
// Profile with special characters in ID (should be sanitized)
1719+
let profile = AppProfile {
1720+
id: "my_profile-123".to_string(),
1721+
name: "Test Profile".to_string(),
1722+
bundle_ids: vec![],
1723+
config_path: PathBuf::new(),
1724+
last_modified: None,
1725+
is_default: false,
1726+
};
1727+
1728+
manager.register_profile(profile).await.unwrap();
1729+
1730+
// Verify config file was created with correct sanitized name
1731+
let expected_path = temp_dir.path().join("my_profile-123.toml");
1732+
assert!(
1733+
expected_path.exists(),
1734+
"Config file should be created with sanitized name: {}",
1735+
expected_path.display()
1736+
);
1737+
}
15301738
}

conductor-gui/ui/src/App.svelte

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import DevicesView from './lib/views/DevicesView.svelte';
66
import ModesView from './lib/views/ModesView.svelte';
77
import MappingsView from './lib/views/MappingsView.svelte';
8+
import ProfilesView from './lib/views/ProfilesView.svelte';
89
import PluginsView from './lib/views/PluginsView.svelte';
910
import SettingsView from './lib/views/SettingsView.svelte';
1011
import { currentSection, restoreNavigationState, SECTIONS } from './lib/stores/navigation.js';
@@ -28,6 +29,8 @@
2829
<ModesView />
2930
{:else if $currentSection === SECTIONS.MAPPINGS}
3031
<MappingsView />
32+
{:else if $currentSection === SECTIONS.PROFILES}
33+
<ProfilesView />
3134
{:else if $currentSection === SECTIONS.PLUGINS}
3235
<PluginsView />
3336
{:else if $currentSection === SECTIONS.SETTINGS}

conductor-gui/ui/src/lib/components/Sidebar.svelte

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
{ id: SECTIONS.DEVICES, label: 'Devices', icon: '🎹' },
66
{ id: SECTIONS.MODES, label: 'Modes', icon: '🎨' },
77
{ id: SECTIONS.MAPPINGS, label: 'Mappings', icon: '🔗' },
8+
{ id: SECTIONS.PROFILES, label: 'App Profiles', icon: '📱' },
89
{ id: SECTIONS.PLUGINS, label: 'Plugins', icon: '🧩' },
910
{ id: SECTIONS.SETTINGS, label: 'Settings', icon: '⚙️' },
1011
];

conductor-gui/ui/src/lib/stores/navigation.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export const SECTIONS = {
1515
DEVICES: 'devices',
1616
MODES: 'modes',
1717
MAPPINGS: 'mappings',
18+
PROFILES: 'profiles', // v4.10.2: App Profiles moved to sidebar (GitHub Issue #53)
1819
PLUGINS: 'plugins',
1920
SETTINGS: 'settings',
2021
};

0 commit comments

Comments
 (0)