From a371a6febc66ad8ef9808b81b41178af85594462 Mon Sep 17 00:00:00 2001 From: Daniel Bodnar <1790726+danielbodnar@users.noreply.github.com> Date: Wed, 18 Mar 2026 21:27:57 -0500 Subject: [PATCH] chore: improve CLAUDE.md with architecture docs, add Claude automations, fix clippy warnings Rewrite CLAUDE.md to document the 5-layer architecture, CLI/TUI dual entry point, provider abstraction, config system, and current project state so new sessions can be productive immediately. Add Claude Code automations: skills (check, test-scaffold), subagents (security-reviewer, clippy-reviewer), context7 MCP server, and hooks for auto-formatting and sensitive file protection. Fix clippy warnings: remove unused imports (StatusCode, error, Path, ProviderType, ProviderCredentials, VolumeStatus, InstanceNetwork, Context, Utc, fs) and unused mut bindings across api/, config/, services/, and main.rs. --- .claude/agents/clippy-reviewer.md | 19 ++++ .claude/agents/security-reviewer.md | 23 +++++ .claude/skills/check.md | 12 +++ .claude/skills/test-scaffold.md | 29 ++++++ .mcp.json | 8 ++ CLAUDE.md | 149 +++++++++++++++++++--------- src/api/proxmox.rs | 4 +- src/api/vyos.rs | 2 +- src/config/credentials.rs | 3 +- src/config/mod.rs | 4 +- src/config/provider.rs | 4 +- src/config/settings.rs | 3 +- src/main.rs | 4 +- src/services/instance.rs | 9 +- src/services/provider.rs | 10 +- src/services/volume.rs | 2 +- 16 files changed, 212 insertions(+), 73 deletions(-) create mode 100644 .claude/agents/clippy-reviewer.md create mode 100644 .claude/agents/security-reviewer.md create mode 100644 .claude/skills/check.md create mode 100644 .claude/skills/test-scaffold.md create mode 100644 .mcp.json diff --git a/.claude/agents/clippy-reviewer.md b/.claude/agents/clippy-reviewer.md new file mode 100644 index 0000000..97f7d18 --- /dev/null +++ b/.claude/agents/clippy-reviewer.md @@ -0,0 +1,19 @@ +--- +name: clippy-reviewer +description: Runs cargo clippy with all warnings and reviews findings after implementation work +--- + +# Clippy Reviewer + +Run `cargo clippy -- -W clippy::all` and analyze the output. + +## Process + +1. Run `cargo clippy -- -W clippy::all 2>&1` +2. Parse warnings and errors +3. For each finding: + - Explain what the lint catches and why it matters + - Suggest the idiomatic fix + - Note if it's a false positive given the project context +4. If there are auto-fixable lints, suggest running `cargo clippy --fix` +5. Report a summary: total warnings, grouped by category diff --git a/.claude/agents/security-reviewer.md b/.claude/agents/security-reviewer.md new file mode 100644 index 0000000..0e245f0 --- /dev/null +++ b/.claude/agents/security-reviewer.md @@ -0,0 +1,23 @@ +--- +name: security-reviewer +description: Reviews code changes in api/, config/credentials.rs, and auth-related code for security issues +--- + +# Security Reviewer + +You are a security-focused code reviewer for bbctl, a CLI/TUI tool that manages infrastructure on VyOS and Proxmox servers. + +## Focus Areas + +- **Credential handling**: Ensure passwords, API keys, and SSH keys are never logged, hardcoded, or exposed in error messages +- **TLS configuration**: Flag any use of `danger_accept_invalid_certs` that isn't behind a configuration flag or warning +- **Command injection**: Check `tokio::process::Command` usage for unsanitized input in SSH commands +- **TOML deserialization**: Verify config parsing handles malicious input gracefully +- **Error messages**: Ensure error output doesn't leak sensitive information (credentials, internal paths) + +## Review Process + +1. Read the changed files (focus on `src/api/`, `src/config/credentials.rs`, `src/services/provider.rs`) +2. Check for the focus areas above +3. Report findings with severity (critical/warning/info) and file:line references +4. If no issues found, confirm the changes look secure diff --git a/.claude/skills/check.md b/.claude/skills/check.md new file mode 100644 index 0000000..c0d741b --- /dev/null +++ b/.claude/skills/check.md @@ -0,0 +1,12 @@ +--- +name: check +description: Run cargo check, clippy, and fmt --check to validate the bbctl codebase +--- + +Run the following commands sequentially, reporting results after each. Stop on first failure: + +1. `cargo fmt --check` — verify formatting +2. `cargo clippy -- -W clippy::all` — lint with all warnings enabled +3. `cargo check` — verify compilation + +Report a summary of any issues found. If all pass, confirm the codebase is clean. diff --git a/.claude/skills/test-scaffold.md b/.claude/skills/test-scaffold.md new file mode 100644 index 0000000..915c7ac --- /dev/null +++ b/.claude/skills/test-scaffold.md @@ -0,0 +1,29 @@ +--- +name: test-scaffold +description: Generate test module for a Rust source file using project conventions (assert_cmd, mockito, tempfile) +disable-model-invocation: true +--- + +# Test Scaffold Generator + +Generate idiomatic Rust tests for a given source file in bbctl. + +## Arguments + +- `$ARGUMENTS` — path to the source file to generate tests for (e.g., `src/config/mod.rs`) + +## Instructions + +1. Read the target source file +2. Identify all public functions, structs, and trait implementations +3. Generate a `#[cfg(test)]` module at the bottom of the file with tests for each public item +4. Use the project's dev-dependencies: + - `tempfile` for tests that need filesystem operations (config files) + - `mockito` for tests that make HTTP requests (API clients) + - `assert_cmd` for CLI integration tests (main.rs) +5. Follow these patterns: + - Test function naming: `test__` + - Use `#[test]` for sync tests, `#[tokio::test]` for async tests + - Include both happy path and error cases + - For config tests, use `tempfile::tempdir()` instead of writing to `~/.bbctl/` +6. Show the generated test module and ask for confirmation before writing diff --git a/.mcp.json b/.mcp.json new file mode 100644 index 0000000..74553cd --- /dev/null +++ b/.mcp.json @@ -0,0 +1,8 @@ +{ + "mcpServers": { + "context7": { + "command": "npx", + "args": ["-y", "@upstash/context7-mcp@latest"] + } + } +} diff --git a/CLAUDE.md b/CLAUDE.md index 18980c7..fa3d554 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,79 +1,130 @@ -# bbctl Development Guidelines +# CLAUDE.md -## Build & Run Commands +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. -```bash -# Build -cargo build +## Build & Test Commands -# Run -cargo run +```bash +cargo build # Dev build +cargo build --release # Release build +cargo run # Launch TUI (no args) +cargo run -- instances list # CLI mode with subcommand +cargo run -- test-vyos --host HOST --port 60022 --username vyos --api-key KEY +cargo test # All tests +cargo test test_name -- --nocapture # Single test with output +cargo fmt # Format +cargo clippy # Lint +``` -# Run with specific command -cargo run -- [command] [subcommand] [args] +## What This Project Is -# Build optimized release version -cargo build --release +bbctl is a CLI/TUI tool for provisioning multi-tenant infrastructure on bare metal servers running VyOS v1.5 or Proxmox. It provides both a Clap-based CLI and an interactive Ratatui terminal dashboard. -# Run specific test -cargo test test_name -- --nocapture +## Architecture -# Format code -cargo fmt +Five-layer design with strict dependency direction (upper layers depend on lower): -# Lint code -cargo clippy +``` +CLI (main.rs) / TUI (app.rs, ui.rs, handler.rs, event.rs, tui.rs) + | + Services (services/) — orchestration, client factory + | + Models (models/) — domain entities with business logic + | + Config (config/) — TOML persistence (~/.bbctl/) + | + API (api/) — provider-specific HTTP/SSH clients ``` -## CLI Examples +### Entry Point (main.rs) -```bash -# List instances -cargo run -- instances list +`#[tokio::main]` dispatches two paths: +- **CLI mode**: `env::args().len() > 1` → `cli_handler()` processes Clap subcommands synchronously +- **TUI mode**: No args → `run_tui()` launches the async Ratatui event loop +- **Special case**: `test-vyos` subcommand is matched before `cli_handler()` because it needs the async runtime for SSH/HTTP calls -# Create instance -cargo run -- instances create my-instance --provider vyos --region nyc --cpu 2 --memory 4 --disk 80 +Clap subcommands: `init`, `deploy`, `instances {list|create|delete|start|stop|show}`, `volumes {list|create|delete|attach|detach|show}`, `networks {list|create|delete|connect|disconnect|show}`, `test-vyos` -# Create volume -cargo run -- volumes create my-volume --size 10 --region nyc +### Provider System (api/) -# Create network -cargo run -- networks create my-network --cidr 192.168.1.0/24 +`Provider` trait in `api/mod.rs` defines the interface (uses `anyhow::Result`): +```rust +pub trait Provider { + fn connect(&self) -> Result<()>; + fn check_connection(&self) -> Result; + fn name(&self) -> &str; +} ``` -## Code Style Guidelines +Two implementations: +- **VyOSClient** (`api/vyos.rs`) — SSH via `tokio::process` + HTTP API with `reqwest`. Key methods: `execute_ssh_command()`, `api_call()`, `get_config()`, `set_config()`, `commit()`, `save()`, `get_system_info()` +- **ProxmoxClient** (`api/proxmox.rs`) — HTTP API with ticket/CSRF auth or API token. Key methods: `login()`, `api_call()`. Auth enum: `ProxmoxAuth::UserPass` or `ProxmoxAuth::ApiToken` + +Both use `reqwest::Client` with `danger_accept_invalid_certs` for self-signed certs. + +### Services Layer (services/) + +`ProviderService` is the factory that bridges config → API clients: +- Loads `Providers` + `Credentials` from TOML +- `get_vyos_client()` / `get_proxmox_client()` instantiate typed clients from stored config + credentials +- `add_vyos_provider()` / `add_proxmox_provider_with_token()` register new providers + +`InstanceService`, `VolumeService`, `NetworkService` each hold in-memory storage (`HashMap`) plus a `ProviderService` reference. + +### Config System (config/) + +Config dir: `~/.bbctl/` (constant `APP_DIR_NAME`). Three TOML files: +- **settings.toml** — global settings (default provider, region, log level) +- **providers.toml** — provider configs + regions (HashMap-based, keyed by name) +- **credentials.toml** — auth credentials, intentionally separated from provider configs + +Helper functions: `get_config_dir()`, `get_config_file()`, `read_config_file()`, `write_config_file()`, `config_file_exists()`, `delete_config_file()`, `init_config()` (creates defaults on first run). -- **Formatting**: Use `cargo fmt` to format code according to Rust standard style +### TUI Architecture -- **Linting**: Run `cargo clippy` for static analysis +Event loop: `App::new()` → `EventHandler` (spawns tokio task reading `crossterm::EventStream` with 250ms tick) → main loop calls `tui.draw()` then matches on `Event::{Tick, Key, Mouse, Resize}`. -- **Naming**: +`AppMode` enum (`Home | Instances | Volumes | Networks | Settings | Help`) drives which `render_*()` function runs. `selected_index` tracks list navigation. Keys: `1-5` jump modes, `j/k` navigate, `q/ESC` back/quit, `?` help. -- Use snake_case for variables, functions, and modules +Note: `app.rs` contains both TUI state (`App`, `AppMode`) and simple display structs (`Instance`, `Volume`, `Network`) used for hardcoded demo data. The richer domain types live in `models/`. -- Use PascalCase for structs, enums, and traits +### Models (models/) -- **Error Handling**: Use `AppResult` for functions that can fail +Domain types with business logic methods (separate from the simpler display structs in `app.rs`): +- **Instance** (`models/instance.rs`) — UUID id, `InstanceStatus` enum, `InstanceSize {cpu, memory_gb, disk_gb}`, `Vec`, tags HashMap +- **Volume** (`models/volume.rs`) — `VolumeStatus`/`VolumeType` enums, `attach()`/`detach()`/`extend()` methods with validation +- **Network** (`models/network.rs`) — `NetworkType` enum, `HashSet` for connected instances, `Vec` with `allocate_ip()`/`release_ip()` +- **Provider** (`models/provider.rs`) — `ProviderType` enum used by other models -- **State Management**: Follow the App/AppMode pattern for managing application state +## Error Handling -- **UI Components**: Use Ratatui components (List, Table, Paragraph) with consistent styling +Two error types coexist: +- `AppResult = Result>` in `app.rs` — used by TUI and main +- `anyhow::Result` — used by config, API, and services layers (with `anyhow::Context` for wrapping) -- **Provider APIs**: VyOS and Proxmox providers should implement common traits +## Key Dependencies -- **Imports**: Group imports by crate, with std first, then external, then internal -- **Document**: Use three slashes (`///`) for public API documentation -- **Async**: Use tokio runtime with futures for async operations +- **ratatui** + **crossterm** (event-stream) — TUI framework +- **clap** (derive) — CLI parsing +- **tokio** (full) — async runtime +- **reqwest** (json, rustls-tls) — HTTP client +- **serde** + **toml** — config serialization +- **uuid** (v4, serde) — resource identifiers +- **anyhow** — error handling in lower layers +- **chrono** (serde) — timestamps on domain models -## Project Structure +## Code Style -- **src/app.rs**: Core application state and data models -- **src/event.rs**: Event handling for TUI (keyboard, mouse, resize) -- **src/handler.rs**: Keyboard event processing -- **src/tui.rs**: Terminal setup and management -- **src/ui.rs**: UI rendering and layout components -- **src/main.rs**: CLI command processing using Clap +- **Formatting**: `cargo fmt` (standard rustfmt) +- **Linting**: `cargo clippy` +- **Naming**: snake_case (vars/fns/modules), PascalCase (structs/enums/traits) +- **Imports**: Group by crate — std first, then external, then internal +- **Docs**: `///` for public API documentation -The app is organized following a typical TUI pattern with app state, event handling, and UI rendering modules. Follow existing patterns when adding new functionality. +## Current State -Future work includes integrating with actual VyOS and Proxmox APIs and adding E2E encryption for public cloud integration. +- CLI handler uses hardcoded println output (not yet connected to real providers) +- TUI displays hardcoded demo data from `App::new()` +- Provider trait only covers connection; no CRUD traits for resources yet +- Services use in-memory `HashMap` storage, no persistence layer +- `models/` has rich domain types but they're not yet wired into `app.rs` display structs diff --git a/src/api/proxmox.rs b/src/api/proxmox.rs index 20ea66d..def2f0d 100644 --- a/src/api/proxmox.rs +++ b/src/api/proxmox.rs @@ -1,8 +1,8 @@ use anyhow::{Result, Context, anyhow}; -use reqwest::{Client, StatusCode}; +use reqwest::Client; use serde::{Deserialize, Serialize}; use std::time::Duration; -use log::{debug, error, info}; +use log::{debug, info}; use crate::api::Provider; diff --git a/src/api/vyos.rs b/src/api/vyos.rs index 2e4d2de..6adf32b 100644 --- a/src/api/vyos.rs +++ b/src/api/vyos.rs @@ -1,5 +1,5 @@ use anyhow::{Result, Context, anyhow}; -use reqwest::{Client, StatusCode}; +use reqwest::Client; use serde::{Deserialize, Serialize}; use std::process::Command; use tokio::process::Command as AsyncCommand; diff --git a/src/config/credentials.rs b/src/config/credentials.rs index 1bda418..e2250e0 100644 --- a/src/config/credentials.rs +++ b/src/config/credentials.rs @@ -1,10 +1,9 @@ use serde::{Deserialize, Serialize}; use anyhow::{Result, Context, anyhow}; use std::collections::HashMap; -use log::{debug, info, error}; +use log::{debug, info}; use crate::config::{read_config_file, write_config_file, CREDENTIALS_FILE}; -use crate::models::provider::ProviderType; /// VyOS credentials #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/src/config/mod.rs b/src/config/mod.rs index 702d61d..3e0c25e 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -2,9 +2,9 @@ pub mod provider; pub mod settings; pub mod credentials; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use anyhow::{Result, Context, anyhow}; -use log::{debug, info, error}; +use log::{debug, info}; use std::fs; use dirs::home_dir; diff --git a/src/config/provider.rs b/src/config/provider.rs index 1b53a35..44a1103 100644 --- a/src/config/provider.rs +++ b/src/config/provider.rs @@ -1,7 +1,7 @@ use serde::{Deserialize, Serialize}; use anyhow::{Result, Context, anyhow}; use std::collections::HashMap; -use log::{debug, info, error}; +use log::{debug, info}; use crate::config::{read_config_file, write_config_file, PROVIDERS_FILE}; use crate::models::provider::{ProviderType, ProviderConfig, Region}; @@ -102,7 +102,7 @@ impl Providers { // Ensure the provider exists let provider_name = region.provider.to_string(); - if !self.providers.iter().any(|(name, p)| p.provider_type == region.provider) { + if !self.providers.iter().any(|(_name, p)| p.provider_type == region.provider) { return Err(anyhow!("Provider '{}' does not exist", provider_name)); } diff --git a/src/config/settings.rs b/src/config/settings.rs index 7ea0149..fe5993e 100644 --- a/src/config/settings.rs +++ b/src/config/settings.rs @@ -1,7 +1,6 @@ use serde::{Deserialize, Serialize}; use anyhow::{Result, Context, anyhow}; -use std::fs; -use log::{debug, info, error}; +use log::{debug, info}; use crate::config::{read_config_file, write_config_file, SETTINGS_FILE}; diff --git a/src/main.rs b/src/main.rs index 8c8517e..78fccad 100644 --- a/src/main.rs +++ b/src/main.rs @@ -290,7 +290,7 @@ fn cli_handler(cli: Cli) -> AppResult<()> { } } } - Some(Commands::TestVyOS { host, port, username, .. }) => { + Some(Commands::TestVyOS { .. }) => { // This would block, so we need to call it outside the CLI handler // Will be implemented in main() return Err("Use tokio runtime to test VyOS connectivity".into()); @@ -378,7 +378,7 @@ async fn main() -> AppResult<()> { println!("\n✅ SSH connection successful!"); // If API key is provided, also test the API - if let Some(api_key) = &api_key { + if api_key.is_some() { println!("\nTesting VyOS HTTP API..."); let mut client_mut = client; diff --git a/src/services/instance.rs b/src/services/instance.rs index 6682c87..c6ac1ea 100644 --- a/src/services/instance.rs +++ b/src/services/instance.rs @@ -3,9 +3,8 @@ use log::{debug, info, error}; use std::collections::HashMap; use uuid::Uuid; use serde_json::json; -use chrono::Utc; -use crate::models::instance::{Instance, InstanceStatus, InstanceSize, InstanceNetwork}; +use crate::models::instance::{Instance, InstanceStatus, InstanceSize}; use crate::models::provider::ProviderType; use crate::services::provider::ProviderService; @@ -246,7 +245,7 @@ impl InstanceService { match provider { ProviderType::VyOS => { // Get VyOS client - let mut client = self.provider_service.get_vyos_client(&provider_name)?; + let client = self.provider_service.get_vyos_client(&provider_name)?; // Use VyOS API to start the VM // Example: Send commands over SSH @@ -320,7 +319,7 @@ impl InstanceService { match provider { ProviderType::VyOS => { // Get VyOS client - let mut client = self.provider_service.get_vyos_client(&provider_name)?; + let client = self.provider_service.get_vyos_client(&provider_name)?; // Use VyOS API to stop the VM // Example: Send commands over SSH @@ -394,7 +393,7 @@ impl InstanceService { match provider { ProviderType::VyOS => { // Get VyOS client - let mut client = self.provider_service.get_vyos_client(&provider_name)?; + let client = self.provider_service.get_vyos_client(&provider_name)?; // Use VyOS API to delete the VM // Example: Send commands over SSH diff --git a/src/services/provider.rs b/src/services/provider.rs index 8bd63fb..c61162f 100644 --- a/src/services/provider.rs +++ b/src/services/provider.rs @@ -1,10 +1,10 @@ -use anyhow::{Result, Context, anyhow}; +use anyhow::{Result, anyhow}; use log::{debug, info, error}; use std::collections::HashMap; use crate::models::provider::{ProviderType, ProviderConfig, Region, ResourceLimits}; use crate::config::provider::Providers; -use crate::config::credentials::{Credentials, ProviderCredentials}; +use crate::config::credentials::Credentials; use crate::api::{Provider, vyos::VyOSClient, vyos::VyOSConfig, proxmox::ProxmoxClient, proxmox::ProxmoxConfig, proxmox::ProxmoxAuth}; /// Provider service for managing infrastructure providers @@ -53,7 +53,7 @@ impl ProviderService { api_port: Option, ) -> Result<()> { // Create provider params - let mut params = HashMap::new(); + let params = HashMap::new(); // Add provider self.providers.add_provider(name, ProviderType::VyOS, host, params)?; @@ -88,7 +88,7 @@ impl ProviderService { verify_ssl: bool, ) -> Result<()> { // Create provider params - let mut params = HashMap::new(); + let params = HashMap::new(); // Add provider self.providers.add_provider(name, ProviderType::Proxmox, host, params)?; @@ -122,7 +122,7 @@ impl ProviderService { verify_ssl: bool, ) -> Result<()> { // Create provider params - let mut params = HashMap::new(); + let params = HashMap::new(); // Add provider self.providers.add_provider(name, ProviderType::Proxmox, host, params)?; diff --git a/src/services/volume.rs b/src/services/volume.rs index 58f16e3..d57d11b 100644 --- a/src/services/volume.rs +++ b/src/services/volume.rs @@ -3,7 +3,7 @@ use log::info; use std::collections::HashMap; use uuid::Uuid; -use crate::models::volume::{Volume, VolumeStatus, VolumeType}; +use crate::models::volume::{Volume, VolumeType}; use crate::models::provider::ProviderType; use crate::services::provider::ProviderService;