feat: implement settings apply command

[kaichen]

Implements issue #8: New Feature: Settings Apply

Add new 'settings apply' subcommand to allow partial application of settings JSON chunks to Claude Code settings. This enables users to manage their settings in modular chunks and apply them from GitHub URLs or local files.

Features

• Apply settings from GitHub URLs (e.g. https://github.com/owner/repo/blob/main/settings.json)
• Apply settings from local files
• --scope option to target user or project settings (defaults to project)
• --overwrite option to replace existing settings (abort by default on conflicts)
• Conflict detection when not using --overwrite
• Atomic file operations for safe settings updates

Closes #8

Generated with Claude Code

[claude]

Claude finished @kaichen's task —— View job

---

Review Complete

Review Tasks

• Gather context and understand the implementation
• Review for logical correctness and algorithm soundness
• Check for potential panics from edge cases or errors
• Identify security vulnerabilities
• Check for performance issues
• Provide focused feedback on showstopper bugs only

Critical Issues Found

1. Potential Panic in GitHub URL Parsing

File: src/commands/settings.rs:98
Severity: High

The convert_to_raw_github_url function can panic on certain edge cases:

let path = parts[4..].join("/");

If a URL like https://github.com/owner/repo/blob/branch is provided (without a file path), accessing parts[4..] will create an empty slice, which is valid but could indicate user error. More critically, the function doesn't handle URLs with query parameters or fragments properly.

Fix: Add bounds checking and handle edge cases:

if parts.len() < 5 {
    return Err(anyhow!("Invalid GitHub URL: missing file path"));
}

2. Network Request Without Timeout

File: src/commands/settings.rs:63
Severity: Medium

The reqwest::get call has no timeout configured, which could hang indefinitely on network issues:

let response = reqwest::get(&raw_url).await

Fix: Configure a reasonable timeout:

let client = reqwest::Client::builder()
    .timeout(std::time::Duration::from_secs(30))
    .build()?;
let response = client.get(&raw_url).send().await

3. Non-Atomic Settings Update

File: src/commands/settings.rs:30-36
Severity: Medium

The pattern of load → merge → save creates a race condition window where concurrent modifications could be lost:

let mut target_settings = load_settings(&target_path)?;
merge_settings(&mut target_settings, source_settings, overwrite)?;
save_settings(&target_path, &target_settings)?;

While save_settings uses atomic file operations internally, the overall operation is not atomic. If two processes run settings apply simultaneously, one could overwrite the other's changes.

Fix: Consider using file locking or accepting this limitation with documentation.

4. Path Traversal Not Fully Mitigated

File: src/commands/settings.rs:104-116
Severity: Low-Medium

While the code checks if the file exists, it doesn't validate that the path is within expected boundaries. A malicious actor could potentially read sensitive files:

let path = PathBuf::from(path);

Fix: Consider canonicalizing paths and validating they're within expected directories, or document that this tool should only be used with trusted input.

Summary

The implementation is generally sound, but the GitHub URL parsing issue could cause panics in production. The network timeout and race condition issues should also be addressed for robustness. The path traversal concern is lower priority but worth considering for defense in depth.

[claude]

Claude finished @kaichen's task —— View job

---

Review Complete

Review Tasks

• Read the settings apply implementation
• Verify if GitHub URL parsing panic issue is addressed
• Check if network timeout is implemented
• Review race condition handling in settings updates
• Assess path traversal security
• Check for any new critical bugs
• Provide focused feedback on showstopper issues only

Review Summary

All previously identified critical issues have been properly addressed:

1. GitHub URL Parsing Panic ✅ FIXED

   • src/commands/settings.rs:165: Added bounds check parts.len() < 5
   • src/commands/settings.rs:177: Added empty path validation
   • No longer susceptible to panic on edge cases

2. Network Timeout ✅ IMPLEMENTED

   • src/commands/settings.rs:116-118: HTTP client configured with 30-second timeout
   • Prevents indefinite hangs on network issues

3. Race Condition ✅ DOCUMENTED

   • src/commands/settings.rs:75-78: Acknowledged as known limitation
   • Reasonable trade-off given infrequent settings modifications
   • Clear documentation for users

4. Path Traversal ✅ MITIGATED

   • src/commands/settings.rs:192-194: Uses canonicalize() to resolve paths
   • src/commands/settings.rs:196-199: Security note about trusted input
   • Appropriate defense for the tool's use case

Verdict

No showstopper bugs found. The implementation is logically sound, handles edge cases properly, and has appropriate security measures. The code is robust against panics and includes proper error handling throughout.

[claude]

Claude finished @kaichen's task —— View job

---

Review Complete

Review Tasks

• Read the latest implementation in src/commands/settings.rs
• Check for potential panics from edge cases
• Verify security measures are maintained
• Check for performance issues
• Review other modified files for critical bugs
• Provide focused feedback on showstopper issues only

Critical Issue Found

1. Potential UTF-8 Panic in agents.rs:155
Severity: High

The string slicing could panic if the description contains multi-byte UTF-8 characters:

if agent_info.description.len() > 80 {
    println!(
        "  {} [{}...]",
        full_agent_name,
        &agent_info.description[..77]  // ❌ Could panic on multi-byte char boundary
    );
}

The code checks len() > 80 (byte length) but slices at [..77] which could split a multi-byte UTF-8 character, causing a panic.

Fix: Use UTF-8 aware truncation:

if agent_info.description.len() > 80 {
    let truncated = agent_info.description
        .chars()
        .take(77)
        .collect::<String>();
    println!("  {} [{}...]", full_agent_name, truncated);
}

Previously Identified Issues - All Fixed ✅

1. GitHub URL parsing panic - FIXED: Proper bounds checking at lines 165-179
2. Network timeout - FIXED: 30-second timeout configured at lines 116-118
3. Race condition - DOCUMENTED: Acknowledged limitation at lines 75-78
4. Path traversal - MITIGATED: Uses canonicalize() at lines 192-194

Summary

Only one showstopper bug found: the UTF-8 string slicing issue in agents.rs that could cause panics when truncating descriptions containing multi-byte characters. All other aspects of the implementation are robust and the previously identified issues have been properly addressed.

[claude]

Claude finished @kaichen's task —— View job

---

Review Complete

Review Tasks

• Verify UTF-8 panic issue in agents.rs is fixed
• Review settings apply implementation for critical bugs
• Check other modified files for panics or security issues
• Provide focused feedback on showstopper issues only

Review Summary

All previously identified critical issues have been successfully resolved:

1. UTF-8 Panic in agents.rs ✅ FIXED

   • Line 152: Now uses agent_info.description.chars().take(77).collect::<String>()
   • This is UTF-8 aware and won't panic on multi-byte character boundaries

2. GitHub URL Parsing ✅ PROPERLY HANDLED

   • settings.rs:165: Validates parts.len() < 5 before accessing array slices
   • settings.rs:177: Checks for empty path after joining
   • No panic risk from out-of-bounds access

3. Network Timeout ✅ IMPLEMENTED

   • settings.rs:116-118: HTTP client configured with 30-second timeout
   • Prevents indefinite hangs on network issues

4. Race Condition ✅ DOCUMENTED

   • settings.rs:75-78: Acknowledged as known limitation with clear comment
   • Reasonable trade-off for this use case

5. Path Traversal ✅ MITIGATED

   • settings.rs:192-194: Uses canonicalize() to resolve paths safely
   • settings.rs:196-199: Security note about trusted input

6. Atomic File Operations ✅ VERIFIED

   • claude.rs: save_settings uses rename for atomic operations
   • Proper cleanup of temporary files on error

Verdict

No showstopper bugs found. The implementation is robust, handles edge cases properly, and has appropriate security measures. All critical issues from previous reviews have been addressed.