From a3adcdfa85ba0e215d19ce15ad25ffcad4c8e6fb Mon Sep 17 00:00:00 2001 From: OKWN Date: Thu, 2 Apr 2026 12:18:41 +0300 Subject: [PATCH 1/9] docs(internal): add ows doctor design note Design document for the `ows doctor` diagnostic command covering: - CLI position (top-level command alongside Update/Uninstall) - V1 check list (vault path, existence, permissions, config/wallet/key/policy parsing) - Internal model (DoctorStatus, DoctorFinding, DoctorReport) - 4-state taxonomy (Pass/Warn/Fail/Skip) with exit code rules - Human-readable output shape - Testing plan and V1 out-of-scope items - File change plan for implementation phases Co-Authored-By: Claude Opus 4.6 --- docs/internal/ows-doctor-plan.md | 203 +++++++++++++++++++++++++++++++ 1 file changed, 203 insertions(+) create mode 100644 docs/internal/ows-doctor-plan.md diff --git a/docs/internal/ows-doctor-plan.md b/docs/internal/ows-doctor-plan.md new file mode 100644 index 00000000..9b545e4f --- /dev/null +++ b/docs/internal/ows-doctor-plan.md @@ -0,0 +1,203 @@ +# `ows doctor` Design Note + +> Internal design document for the `ows doctor` diagnostic command. + +## Overview + +`ows doctor` is a read-only diagnostic command that inspects the local OWS installation and vault health. It does not mutate, repair, or modify any files. + +## 1. CLI Position + +`ows doctor` is a **top-level command** in `Commands`, positioned alongside `Update` and `Uninstall`: + +```rust +/// In main.rs +enum Commands { + // ... existing ... + Doctor, +} +``` + +Handler dispatches to `commands::doctor::run()`. + +## 2. V1 Checks + +| Check ID | Name | Description | +|----------|------|-------------| +| `vault_path` | Vault Path Resolution | Vault path derived from `Config::default()` | +| `vault_exists` | Vault Existence | Whether `~/.ows` directory exists | +| `wallets_dir` | Wallets Directory | Whether `wallets/` subdirectory exists and is readable | +| `config_parse` | Config Parse | Whether `config.json` exists and parses as `Config` | +| `wallet_files` | Wallet Files | Enumerate all `.json` files in `wallets/`, parse each as `EncryptedWallet` | +| `key_files` | Key Files | Enumerate all `.json` files in `keys/`, parse each as `ApiKeyFile` | +| `policy_files` | Policy Files | Enumerate all `.json` files in `policies/`, parse each as `Policy` | +| `vault_permissions` | Vault Permissions | Check directory and file permissions (Unix only, skip on Windows) | +| `runtime_deps` | Runtime Dependencies | Check for OpenSSL via `pkg-config` (Unix only) | +| `home_env` | HOME Environment | Whether `HOME` env var is set | + +## 3. Internal Data Model + +```rust +/// Status of a single check. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DoctorStatus { + Pass, // Check succeeded + Warn, // Check passed but with concerns + Fail, // Check failed + Skip, // Check skipped (not applicable, e.g., no wallets yet) +} + +/// A single diagnostic check result. +#[derive(Debug)] +pub struct DoctorFinding { + pub id: &'static str, + pub status: DoctorStatus, + pub message: String, + pub remediation: Option, +} + +/// Aggregated diagnostic report. +#[derive(Debug, Default)] +pub struct DoctorReport { + pub findings: Vec, +} +``` + +**Key design decisions:** +- `Skip` is distinct from `Pass` to avoid alarming users about normal states (e.g., no wallets yet) +- `remediation` is `Option` — present for `Warn` and `Fail`, absent for `Pass` and `Skip` +- No severity enumeration (OK/WARN/ERROR/INFO) — V1 uses `Pass/Warn/Fail/Skip` only +- Report is just a collection of findings; grouping/sorting happens at display time + +## 4. Warning/Error Taxonomy + +V1 uses a **4-state model**: + +| State | Meaning | Exit Code Impact | +|-------|---------|------------------| +| `Pass` | Check succeeded, no issues | Continue (0) | +| `Skip` | Check not applicable (e.g., no wallets) | Continue (0) | +| `Warn` | Minor concern detected | Continue (0) | +| `Fail` | Critical issue detected | Exit (1) | + +**Rules:** +- If **any** `Fail` exists → exit code 1 +- Otherwise → exit code 0 + +## 5. Output Shape (Human-Readable) + +``` +OWS Doctor +========== + +[Pass] vault_path Vault resolved to ~/.ows +[Pass] vault_exists ~/.ows exists +[Pass] wallets_dir wallets/ present (2 wallets found) +[Pass] config_parse config.json valid (15 RPC endpoints) +[Pass] policy_files policies/ valid (1 policy) +[Skip] key_files keys/ not present +[Pass] vault_perms Permissions correct +[Pass] home_env HOME is set +[Pass] runtime_deps OpenSSL available + +Summary: 7 passed, 0 warnings, 0 failed, 2 skipped +``` + +**With warnings:** +``` +[Pass] vault_path Vault resolved to ~/.ows +[Pass] vault_exists ~/.ows exists +[Pass] wallets_dir wallets/ present (2 wallets found) +[Pass] config_parse config.json valid +[Warn] vault_perms ~/.ows has mode 0755, expected 0700 + → Run: chmod 700 ~/.ows + +Summary: 4 passed, 1 warning, 0 failed, 0 skipped +``` + +**With failures:** +``` +[Fail] wallet_files wallet-abc.json: JSON parse error at line 3 + → Backup and recreate the wallet + +Summary: 3 passed, 0 warnings, 1 failed, 0 skipped +``` + +**Key output rules:** +- No emoji in the check list (emoji only in summary banner) +- Remediation on the line below, indented with `→ ` +- No color codes (not a terminal UI feature) +- Grouped by status in output: Fail first, then Warn, then Pass, then Skip + +## 6. Testing Plan + +| Test | Scope | +|------|-------| +| `test_doctor_report_default` | Empty vault — all findings are Skip except vault_path/vault_exists | +| `test_doctor_report_missing_home` | HOME unset — `home_env` returns Warn | +| `test_doctor_report_permission_warning` | Vault dir mode 0755 — `vault_perms` returns Warn | +| `test_doctor_report_wallet_corrupted` | Invalid JSON in wallet file — `wallet_files` returns Fail | +| `test_doctor_report_multiple_findings` | Several findings at once — summary counts are correct | +| `test_doctor_exit_code_pass` | All findings Pass/Skip → exit 0 | +| `test_doctor_exit_code_fail` | Any Fail → exit 1 | +| `test_doctor_exit_code_warn_only` | Warn only → exit 0 | + +Tests live in `ows-cli/src/commands/doctor.rs` under `#[cfg(test)]`. + +## 7. V1 Out of Scope + +The following are explicitly **not** included in V1: + +- **File repair or mutation** — no `chmod`, no backup, no auto-fix +- **Wallet balance or RPC connectivity checks** — network calls; out of scope for local diagnostics +- **Deterministic JSON output** — human-readable only in V1; structured output (e.g., `--json`) is V2 +- **Per-wallet detailed report** — V1 shows counts only; individual wallet health is V2 +- **Policy rule validation** — V1 checks file parseability only; rule evaluation is V2 +- **Cross-vault migration checks** — no checking of `~/.lws` vs `~/.ows` +- **Dependency version checking** — `cargo`, `git`, Node.js version detection + +## 8. File Changes Plan + +| File | Change | +|------|--------| +| `ows/crates/ows-cli/src/commands/doctor.rs` | **New** — all diagnostic logic | +| `ows/crates/ows-cli/src/commands/mod.rs` | Add `pub mod doctor;` | +| `ows/crates/ows-cli/src/main.rs` | Add `Doctor` variant + handler | +| `docs/sdk-cli.md` | Add `### ows doctor` section under System Commands | + +## 9. Crate Boundaries + +``` +ows-cli (command layer) + └── calls: ows_lib::vault, ows_lib::policy_store + └── calls: ows_core::Config, ows_core::EncryptedWallet, ows_core::Policy, ows_core::ApiKeyFile + └── calls: ows_signer (no-op in doctor, just re-exports) + +ows-lib (storage layer) + └── vault.rs: resolve_vault_path(), wallets_dir(), list_encrypted_wallets() + └── policy_store.rs: policies_dir(), list_policies() + └── key_ops.rs: keys_dir() — NOT YET EXPOSED; may need to add + +ows-core (types layer) + └── Config, EncryptedWallet, Policy, ApiKeyFile +``` + +**Note:** `keys_dir()` is not currently exposed from `ows-lib`. It must be added to `ows-lib/src/vault.rs` and re-exported in `ows-lib/src/lib.rs` before `key_files` check can enumerate key files. + +## 10. Implementation Phases + +**Phase 1 (this PR):** +- Core model: `DoctorStatus`, `DoctorFinding`, `DoctorReport` +- All checks except `key_files` +- Human-readable output +- Unit tests +- Docs + +**Phase 2 (future):** +- Add `keys_dir()` to `ows-lib` +- `key_files` check +- `--json` structured output flag + +**Phase 3 (future):** +- Per-wallet detailed report +- Policy rule validation From 09e7490d884542446dbc03f24a2774e6b8e395e6 Mon Sep 17 00:00:00 2001 From: OKWN Date: Thu, 2 Apr 2026 12:41:07 +0300 Subject: [PATCH 2/9] feat(ows-cli): add diagnostic model and check framework for `ows doctor` Phase 2: Internal diagnostic domain model and execution framework (CLI wiring comes in Phase 3) - `report.rs`: DoctorStatus (Ok/Warning/Error/Skipped), DoctorFinding (id, status, title, detail, suggestion, path, code), DoctorReport (findings, summary counts, overall_status, exit_code), DoctorCheckId - `checks.rs`: Individual check implementations for vault path, vault existence, wallets/keys/policies/logs subdirectories, config parse, Unix permissions. Each check is a pure function returning Vec. resolve_vault_path() helper avoids env var dependence in tests. - `mod.rs`: Module docs, re-exports, DoctorResult alias - `commands/mod.rs`: Added `pub mod doctor;` - 16 passing unit tests covering: empty vault, missing subdirs, malformed config, permission warnings, healthy state, report aggregation and exit code logic Co-Authored-By: Claude Opus 4.6 --- .../ows-cli/src/commands/doctor/checks.rs | 598 ++++++++++++++++++ ows/crates/ows-cli/src/commands/doctor/mod.rs | 33 + .../ows-cli/src/commands/doctor/report.rs | 329 ++++++++++ ows/crates/ows-cli/src/commands/mod.rs | 1 + 4 files changed, 961 insertions(+) create mode 100644 ows/crates/ows-cli/src/commands/doctor/checks.rs create mode 100644 ows/crates/ows-cli/src/commands/doctor/mod.rs create mode 100644 ows/crates/ows-cli/src/commands/doctor/report.rs diff --git a/ows/crates/ows-cli/src/commands/doctor/checks.rs b/ows/crates/ows-cli/src/commands/doctor/checks.rs new file mode 100644 index 00000000..3b5ac57f --- /dev/null +++ b/ows/crates/ows-cli/src/commands/doctor/checks.rs @@ -0,0 +1,598 @@ +//! Individual diagnostic checks for `ows doctor`. + +use crate::commands::doctor::report::{DoctorCheckId, DoctorFinding, DoctorReport}; + +use ows_core::Config; + +// --------------------------------------------------------------------------- +// Check IDs +// --------------------------------------------------------------------------- + +/// Vault path resolution check ID. +pub const CHECK_VAULT_PATH: DoctorCheckId = DoctorCheckId::new("vault.path"); +/// Vault existence check ID. +pub const CHECK_VAULT_EXISTS: DoctorCheckId = DoctorCheckId::new("vault.exists"); +/// Wallets directory presence check ID. +pub const CHECK_WALLETS_DIR: DoctorCheckId = DoctorCheckId::new("vault.wallets_dir"); +/// Keys directory presence check ID. +pub const CHECK_KEYS_DIR: DoctorCheckId = DoctorCheckId::new("vault.keys_dir"); +/// Policies directory presence check ID. +pub const CHECK_POLICIES_DIR: DoctorCheckId = DoctorCheckId::new("vault.policies_dir"); +/// Logs directory presence check ID. +pub const CHECK_LOGS_DIR: DoctorCheckId = DoctorCheckId::new("vault.logs_dir"); +/// Config file presence and parseability check ID. +pub const CHECK_CONFIG: DoctorCheckId = DoctorCheckId::new("config.parse"); +/// Vault directory permissions check ID (Unix only). +pub const CHECK_VAULT_PERMS: DoctorCheckId = DoctorCheckId::new("vault.permissions"); +/// HOME environment variable check ID. +pub const CHECK_HOME_ENV: DoctorCheckId = DoctorCheckId::new("env.home"); + +// --------------------------------------------------------------------------- +// Vault path resolution +// --------------------------------------------------------------------------- + +/// Resolve the vault path, using an override for testing purposes. +/// +/// When `vault_path_override` is `None`, resolves from `Config::default()`. +/// Tests can pass a specific path to avoid environment variable dependence. +fn resolve_vault_path(vault_path_override: Option<&std::path::Path>) -> std::path::PathBuf { + match vault_path_override { + Some(p) => p.to_path_buf(), + None => Config::default().vault_path, + } +} + +// --------------------------------------------------------------------------- +// Check implementations +// --------------------------------------------------------------------------- + +/// Check that HOME is set and the vault path resolves correctly. +pub fn check_vault_path() -> Vec { + let mut findings = Vec::new(); + + let home = std::env::var("HOME").ok(); + if home.is_none() { + findings.push(DoctorFinding::error( + CHECK_HOME_ENV, + "HOME not set", + "The HOME environment variable is not set. Vault path resolution may be incorrect.", + "Set HOME to your user directory path.", + )); + } + + let vault_path = resolve_vault_path(None); + + findings.push(DoctorFinding::ok( + CHECK_VAULT_PATH, + "Vault path resolved", + &format!("Vault path resolved to `{}`", vault_path.display()), + )); + + findings +} + +/// Check that the vault directory exists. +pub fn check_vault_exists() -> Vec { + let vault_path = resolve_vault_path(None); + + if vault_path.exists() { + vec![DoctorFinding::ok( + CHECK_VAULT_EXISTS, + "Vault exists", + &format!("`{}` exists", vault_path.display()), + )] + } else { + vec![DoctorFinding::error( + CHECK_VAULT_EXISTS, + "Vault not found", + &format!( + "Vault directory not found at `{}`. No wallets have been created yet.", + vault_path.display() + ), + "Run `ows wallet create` to create your first wallet.", + ) + .with_path(vault_path.clone())] + } +} + +/// Check that the wallets subdirectory exists (if vault exists). +pub fn check_wallets_dir() -> Vec { + let vault_path = resolve_vault_path(None); + + if !vault_path.exists() { + return vec![DoctorFinding::skipped( + CHECK_WALLETS_DIR, + "Wallets directory skipped", + "Vault does not exist; skipping wallets directory check.", + )]; + } + + let wallets_dir = vault_path.join("wallets"); + + if wallets_dir.exists() { + // Count wallet files + let wallet_count = std::fs::read_dir(&wallets_dir) + .map(|entries| { + entries + .filter_map(|e| e.ok()) + .filter(|e| e.path().extension().and_then(|s| s.to_str()) == Some("json")) + .count() + }) + .unwrap_or(0); + + vec![DoctorFinding::ok( + CHECK_WALLETS_DIR, + "Wallets directory present", + &format!( + "wallets/ exists with {} wallet file(s)", + wallet_count + ), + ) + .with_path(wallets_dir)] + } else { + vec![DoctorFinding::error( + CHECK_WALLETS_DIR, + "Wallets directory missing", + &format!( + "Expected `{}` but it does not exist.", + wallets_dir.display() + ), + "This is unexpected. The wallet command should create this directory.", + ) + .with_path(wallets_dir)] + } +} + +/// Check that the keys subdirectory exists (if vault exists). +pub fn check_keys_dir() -> Vec { + let vault_path = resolve_vault_path(None); + + if !vault_path.exists() { + return vec![DoctorFinding::skipped( + CHECK_KEYS_DIR, + "Keys directory skipped", + "Vault does not exist; skipping keys directory check.", + )]; + } + + let keys_dir = vault_path.join("keys"); + + if keys_dir.exists() { + vec![DoctorFinding::ok( + CHECK_KEYS_DIR, + "Keys directory present", + &format!("keys/ exists at `{}`", keys_dir.display()), + ) + .with_path(keys_dir)] + } else { + vec![DoctorFinding::skipped( + CHECK_KEYS_DIR, + "Keys directory not present", + "keys/ does not exist. No API keys have been created yet.", + )] + } +} + +/// Check that the policies subdirectory exists (if vault exists). +pub fn check_policies_dir() -> Vec { + let vault_path = resolve_vault_path(None); + + if !vault_path.exists() { + return vec![DoctorFinding::skipped( + CHECK_POLICIES_DIR, + "Policies directory skipped", + "Vault does not exist; skipping policies directory check.", + )]; + } + + let policies_dir = vault_path.join("policies"); + + if policies_dir.exists() { + vec![DoctorFinding::ok( + CHECK_POLICIES_DIR, + "Policies directory present", + &format!("policies/ exists at `{}`", policies_dir.display()), + ) + .with_path(policies_dir)] + } else { + vec![DoctorFinding::skipped( + CHECK_POLICIES_DIR, + "Policies directory not present", + "policies/ does not exist. No policies have been created yet.", + )] + } +} + +/// Check that the logs subdirectory exists (if vault exists). +pub fn check_logs_dir() -> Vec { + let vault_path = resolve_vault_path(None); + + if !vault_path.exists() { + return vec![DoctorFinding::skipped( + CHECK_LOGS_DIR, + "Logs directory skipped", + "Vault does not exist; skipping logs directory check.", + )]; + } + + let logs_dir = vault_path.join("logs"); + + if logs_dir.exists() { + vec![DoctorFinding::ok( + CHECK_LOGS_DIR, + "Logs directory present", + &format!("logs/ exists at `{}`", logs_dir.display()), + ) + .with_path(logs_dir)] + } else { + vec![DoctorFinding::skipped( + CHECK_LOGS_DIR, + "Logs directory not present", + "logs/ does not exist. Audit logging may not be active.", + )] + } +} + +/// Check that the config file is present and parseable. +pub fn check_config() -> Vec { + let config_path = resolve_vault_path(None).join("config.json"); + + if !config_path.exists() { + return vec![DoctorFinding::skipped( + CHECK_CONFIG, + "Config file not present", + "No user config file at `~/.ows/config.json`. Using built-in defaults.", + )]; + } + + // Use Config::load to get proper error handling for malformed JSON + match Config::load(&config_path) { + Ok(config) => { + let rpc_count = config.rpc.len(); + vec![DoctorFinding::ok( + CHECK_CONFIG, + "Config valid", + &format!( + "config.json is valid with {} RPC endpoint(s) configured", + rpc_count + ), + ) + .with_path(config_path)] + } + Err(e) => { + vec![DoctorFinding::error( + CHECK_CONFIG, + "Config parse error", + &format!("config.json exists but failed to parse: {}", e), + "Backup and recreate `~/.ows/config.json`.", + ) + .with_path(config_path) + .with_code("ERR_CONFIG_PARSE")] + } + } +} + +/// Check vault directory permissions (Unix only). +#[cfg(unix)] +pub fn check_vault_permissions() -> Vec { + use std::os::unix::fs::PermissionsExt; + + let vault_path = resolve_vault_path(None); + + if !vault_path.exists() { + return vec![DoctorFinding::skipped( + CHECK_VAULT_PERMS, + "Permissions check skipped", + "Vault does not exist; skipping permissions check.", + )]; + } + + let mut findings = Vec::new(); + + // Check vault directory permissions + if let Ok(meta) = std::fs::metadata(vault_path) { + let mode = meta.permissions().mode() & 0o777; + if mode != 0o700 { + findings.push( + DoctorFinding::warning( + CHECK_VAULT_PERMS, + "Vault permissions too open", + &format!( + "Vault directory has permissions {:03o}, expected 0700 (owner-only)", + mode + ), + "Run: chmod 700 ~/.ows", + ) + .with_path(vault_path.clone()) + .with_code("WARN_VAULT_PERMS"), + ); + } else { + findings.push( + DoctorFinding::ok( + CHECK_VAULT_PERMS, + "Vault permissions correct", + "Vault directory has correct permissions (0700).", + ) + .with_path(vault_path.clone()), + ); + } + } + + // Check wallets directory permissions + let wallets_dir = vault_path.join("wallets"); + if let Ok(meta) = std::fs::metadata(&wallets_dir) { + let mode = meta.permissions().mode() & 0o777; + if mode != 0o700 { + findings.push( + DoctorFinding::warning( + CHECK_VAULT_PERMS, + "Wallets directory permissions too open", + &format!( + "Wallets directory has permissions {:03o}, expected 0700", + mode + ), + "Run: chmod 700 ~/.ows/wallets", + ) + .with_path(wallets_dir.clone()) + .with_code("WARN_WALLETS_PERMS"), + ); + } + } + + // Check wallet file permissions + if let Ok(entries) = std::fs::read_dir(&wallets_dir) { + for entry in entries.filter_map(|e| e.ok()) { + if entry.path().extension().and_then(|s| s.to_str()) == Some("json") { + if let Ok(meta) = std::fs::metadata(entry.path()) { + let mode = meta.permissions().mode() & 0o777; + if mode != 0o600 { + let file_name = entry.file_name().to_string_lossy(); + findings.push( + DoctorFinding::warning( + CHECK_VAULT_PERMS, + "Wallet file permissions too open", + &format!( + "{} has permissions {:03o}, expected 0600", + file_name, mode + ), + &format!("Run: chmod 600 ~/.ows/wallets/{}", file_name), + ) + .with_path(entry.path()) + .with_code("WARN_WALLET_FILE_PERMS"), + ); + } + } + } + } + } + + if findings.is_empty() { + // No findings means we didn't add anything above + vec![DoctorFinding::ok( + CHECK_VAULT_PERMS, + "Permissions check passed", + "All vault and wallet permissions are correct.", + )] + } else { + findings + } +} + +/// Check vault directory permissions (Windows stub — no-op). +#[cfg(not(unix))] +pub fn check_vault_permissions() -> Vec { + vec![DoctorFinding::skipped( + CHECK_VAULT_PERMS, + "Permissions check skipped", + "Permission checks are Unix-only.", + )] +} + +// --------------------------------------------------------------------------- +// Check runner +// --------------------------------------------------------------------------- + +/// Run all V1 diagnostic checks and return the aggregated report. +/// +/// Checks run in a fixed order. Each check is independent and produces +/// zero or more findings. All findings are collected into a single report. +pub fn run_all_checks() -> DoctorReport { + let mut all_findings = Vec::new(); + + // Path resolution and HOME check + all_findings.extend(check_vault_path()); + + // Vault existence + all_findings.extend(check_vault_exists()); + + // Required directories + all_findings.extend(check_wallets_dir()); + all_findings.extend(check_keys_dir()); + all_findings.extend(check_policies_dir()); + all_findings.extend(check_logs_dir()); + + // Config + all_findings.extend(check_config()); + + // Permissions (platform-specific) + all_findings.extend(check_vault_permissions()); + + DoctorReport::new(all_findings) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::commands::doctor::DoctorStatus; + use tempfile::TempDir; + + #[test] + fn test_check_vault_path_includes_home_check() { + let findings = check_vault_path(); + // Should have at least 2 findings: HOME check + vault path + assert!(findings.len() >= 2); + + // Should have CHECK_VAULT_PATH finding + let path_finding = findings.iter().find(|f| f.id == CHECK_VAULT_PATH); + assert!(path_finding.is_some()); + assert_eq!(path_finding.unwrap().status, DoctorStatus::Ok); + } + + #[test] + fn test_check_vault_exists_not_found() { + // Use a path that definitely doesn't exist + let temp_dir = TempDir::new().unwrap(); + std::env::set_var("HOME", temp_dir.path()); + + let findings = check_vault_exists(); + assert_eq!(findings.len(), 1); + let finding = &findings[0]; + assert_eq!(finding.id, CHECK_VAULT_EXISTS); + assert_eq!(finding.status, DoctorStatus::Error); + assert!(finding.detail.contains("not found")); + } + + #[test] + fn test_check_wallets_dir_skipped_when_vault_missing() { + let temp_dir = TempDir::new().unwrap(); + std::env::set_var("HOME", temp_dir.path()); + + let findings = check_wallets_dir(); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].status, DoctorStatus::Skipped); + } + + #[test] + fn test_check_keys_dir_skipped_when_vault_missing() { + let temp_dir = TempDir::new().unwrap(); + std::env::set_var("HOME", temp_dir.path()); + + let findings = check_keys_dir(); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].status, DoctorStatus::Skipped); + } + + #[test] + fn test_check_policies_dir_skipped_when_vault_missing() { + let temp_dir = TempDir::new().unwrap(); + std::env::set_var("HOME", temp_dir.path()); + + let findings = check_policies_dir(); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].status, DoctorStatus::Skipped); + } + + #[test] + fn test_check_logs_dir_skipped_when_vault_missing() { + let temp_dir = TempDir::new().unwrap(); + std::env::set_var("HOME", temp_dir.path()); + + let findings = check_logs_dir(); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].status, DoctorStatus::Skipped); + } + + #[test] + fn test_check_config_skipped_when_no_config() { + let temp_dir = TempDir::new().unwrap(); + std::env::set_var("HOME", temp_dir.path()); + + let findings = check_config(); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].id, CHECK_CONFIG); + assert_eq!(findings[0].status, DoctorStatus::Skipped); + } + + #[test] + fn test_check_config_valid() { + let temp_dir = TempDir::new().unwrap(); + std::env::set_var("HOME", temp_dir.path()); + + // Create a valid config + let config_path = temp_dir.path().join(".ows/config.json"); + std::fs::create_dir_all(config_path.parent().unwrap()).ok(); + let config_content = serde_json::json!({ + "vault_path": "/test/.ows", + "rpc": { + "eip155:1": "https://eth.example.com" + } + }); + std::fs::write(&config_path, serde_json::to_string_pretty(&config_content).unwrap()).ok(); + + let findings = check_config(); + assert_eq!(findings.len(), 1); + let finding = &findings[0]; + assert_eq!(finding.id, CHECK_CONFIG); + assert_eq!(finding.status, DoctorStatus::Ok); + assert!(finding.detail.contains("1 RPC endpoint")); + } + + #[test] + fn test_check_config_malformed() { + let temp_dir = TempDir::new().unwrap(); + std::env::set_var("HOME", temp_dir.path()); + + // Create a malformed config + let config_path = temp_dir.path().join(".ows/config.json"); + std::fs::create_dir_all(config_path.parent().unwrap()).ok(); + std::fs::write(&config_path, "{ invalid json }").ok(); + + let findings = check_config(); + assert_eq!(findings.len(), 1); + let finding = &findings[0]; + assert_eq!(finding.id, CHECK_CONFIG); + assert_eq!(finding.status, DoctorStatus::Error); + assert!(finding.code.is_some()); + } + + #[test] + fn test_run_all_checks_returns_valid_report() { + // Use a temporary HOME so we don't affect real vault + let temp_dir = TempDir::new().unwrap(); + std::env::set_var("HOME", temp_dir.path()); + + let report = run_all_checks(); + + // Should have findings + assert!(!report.findings.is_empty()); + + // Should have an overall status + assert!(matches!( + report.overall_status, + DoctorStatus::Ok | DoctorStatus::Warning | DoctorStatus::Error | DoctorStatus::Skipped + )); + + // Exit code should be 0 or 1 + assert!(report.exit_code() >= 0 && report.exit_code() <= 1); + } + + #[test] + fn test_check_wallets_dir_with_wallets() { + let temp_dir = TempDir::new().unwrap(); + std::env::set_var("HOME", temp_dir.path()); + + // Create vault with wallets + let vault = temp_dir.path().join(".ows"); + let wallets_dir = vault.join("wallets"); + std::fs::create_dir_all(&wallets_dir).ok(); + + // Add a dummy wallet file + let wallet_content = serde_json::json!({ + "ows_version": 2, + "id": "test-wallet", + "name": "Test Wallet", + "created_at": "2026-01-01T00:00:00Z", + "accounts": [], + "crypto": {}, + "key_type": "mnemonic" + }); + let wallet_path = wallets_dir.join("test-wallet.json"); + std::fs::write(&wallet_path, serde_json::to_string_pretty(&wallet_content).unwrap()).ok(); + + let findings = check_wallets_dir(); + assert_eq!(findings.len(), 1); + let finding = &findings[0]; + assert_eq!(finding.status, DoctorStatus::Ok); + assert!(finding.detail.contains("1 wallet file")); + } +} diff --git a/ows/crates/ows-cli/src/commands/doctor/mod.rs b/ows/crates/ows-cli/src/commands/doctor/mod.rs new file mode 100644 index 00000000..14d87c30 --- /dev/null +++ b/ows/crates/ows-cli/src/commands/doctor/mod.rs @@ -0,0 +1,33 @@ +//! `ows doctor` — Diagnostic command for OWS installation health. +//! +//! This module provides a read-only diagnostic system that checks: +//! - Vault path resolution +//! - Vault and subdirectory existence +//! - Config file validity +//! - File permissions (Unix) +//! - Environment configuration +//! +//! All checks are read-only and do not modify any files. +//! +//! # Architecture +//! +//! - [`report`] — Domain types: `DoctorStatus`, `DoctorFinding`, `DoctorReport` +//! - [`checks`] — Individual check implementations +//! - [`run_all_checks()`] — Aggregates all findings into a report +//! +//! # Stability +//! +//! The check IDs, status enums, and report structure are considered stable +//! and will not change in a breaking way. Output formatting is separate +//! and can evolve independently. + +pub mod checks; +pub mod report; + +// Re-exported for use by the CLI command (Phase 3) and integration tests. +#[allow(unused)] +pub use report::{DoctorCheckId, DoctorFinding, DoctorReport, DoctorStatus, DoctorSummary}; +pub use checks::run_all_checks; + +/// Aggregated diagnostic report — the output of [`run_all_checks()`]. +pub type DoctorResult = DoctorReport; diff --git a/ows/crates/ows-cli/src/commands/doctor/report.rs b/ows/crates/ows-cli/src/commands/doctor/report.rs new file mode 100644 index 00000000..476ad01a --- /dev/null +++ b/ows/crates/ows-cli/src/commands/doctor/report.rs @@ -0,0 +1,329 @@ +//! Diagnostic report types for `ows doctor`. + +use std::fmt; + +/// Unique identifier for a diagnostic check. +/// +/// Codes are stable, human-readable identifiers used for: +/// - Structured output (future JSON mode) +/// - Test identification +/// - Cross-referencing findings +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct DoctorCheckId(&'static str); + +impl DoctorCheckId { + pub const fn new(code: &'static str) -> Self { + DoctorCheckId(code) + } + + pub fn as_str(&self) -> &'static str { + self.0 + } +} + +impl fmt::Display for DoctorCheckId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} + +/// Status of a single diagnostic check. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[non_exhaustive] +pub enum DoctorStatus { + /// Check succeeded with no issues. + Ok, + /// Check passed but a minor concern was detected. + Warning, + /// Check failed; a problem requires attention. + Error, + /// Check was skipped because it does not apply (e.g., no wallets exist). + Skipped, +} + +impl DoctorStatus { + pub fn is_passing(&self) -> bool { + matches!(self, DoctorStatus::Ok | DoctorStatus::Skipped) + } +} + +/// A single diagnostic finding from one check. +#[derive(Debug, Clone)] +pub struct DoctorFinding { + /// Unique identifier for the check that produced this finding. + pub id: DoctorCheckId, + /// Status of the check. + pub status: DoctorStatus, + /// Short title for the finding (suitable for display). + pub title: String, + /// Detailed explanation of the finding. + pub detail: String, + /// Actionable suggestion for remediation, if applicable. + pub suggestion: Option, + /// Path to the file or directory involved, if applicable. + pub path: Option, + /// Stable error code for machine processing (future JSON output). + pub code: Option<&'static str>, +} + +impl DoctorFinding { + pub fn ok(id: DoctorCheckId, title: &str, detail: &str) -> Self { + DoctorFinding { + id, + status: DoctorStatus::Ok, + title: title.to_string(), + detail: detail.to_string(), + suggestion: None, + path: None, + code: None, + } + } + + pub fn skipped(id: DoctorCheckId, title: &str, detail: &str) -> Self { + DoctorFinding { + id, + status: DoctorStatus::Skipped, + title: title.to_string(), + detail: detail.to_string(), + suggestion: None, + path: None, + code: None, + } + } + + pub fn warning(id: DoctorCheckId, title: &str, detail: &str, suggestion: &str) -> Self { + DoctorFinding { + id, + status: DoctorStatus::Warning, + title: title.to_string(), + detail: detail.to_string(), + suggestion: Some(suggestion.to_string()), + path: None, + code: None, + } + } + + pub fn error(id: DoctorCheckId, title: &str, detail: &str, suggestion: &str) -> Self { + DoctorFinding { + id, + status: DoctorStatus::Error, + title: title.to_string(), + detail: detail.to_string(), + suggestion: Some(suggestion.to_string()), + path: None, + code: None, + } + } + + /// Builder-style method to attach a path to the finding. + pub fn with_path(mut self, path: std::path::PathBuf) -> Self { + self.path = Some(path); + self + } + + /// Builder-style method to attach an error code to the finding. + pub fn with_code(mut self, code: &'static str) -> Self { + self.code = Some(code); + self + } +} + +/// Summary counts across all findings. +#[derive(Debug, Clone, Default)] +pub struct DoctorSummary { + pub ok: usize, + pub warnings: usize, + pub errors: usize, + pub skipped: usize, +} + +impl DoctorSummary { + pub fn total(&self) -> usize { + self.ok + self.warnings + self.errors + self.skipped + } + + pub fn has_failures(&self) -> bool { + self.errors > 0 + } +} + +/// Aggregated diagnostic report from all checks. +#[derive(Debug, Clone)] +pub struct DoctorReport { + /// The most severe status across all findings. + pub overall_status: DoctorStatus, + /// All individual findings in the order they were produced. + pub findings: Vec, + /// Summary counts. + pub summary: DoctorSummary, +} + +impl DoctorReport { + /// Create a new report from a list of findings. + /// + /// Findings are preserved in order. Overall status is derived as: + /// - `Error` if any error exists + /// - `Warning` if any warning exists (and no errors) + /// - `Ok` if only ok/skipped findings + /// - `Skipped` if only skipped findings + pub fn new(findings: Vec) -> Self { + let summary = DoctorSummary { + ok: findings.iter().filter(|f| f.status == DoctorStatus::Ok).count(), + warnings: findings.iter().filter(|f| f.status == DoctorStatus::Warning).count(), + errors: findings.iter().filter(|f| f.status == DoctorStatus::Error).count(), + skipped: findings.iter().filter(|f| f.status == DoctorStatus::Skipped).count(), + }; + + let overall_status = if summary.errors > 0 { + DoctorStatus::Error + } else if summary.warnings > 0 { + DoctorStatus::Warning + } else if summary.ok == 0 && summary.skipped > 0 { + DoctorStatus::Skipped + } else { + DoctorStatus::Ok + }; + + DoctorReport { + overall_status, + findings, + summary, + } + } + + /// Return true if the report indicates any errors. + pub fn has_errors(&self) -> bool { + self.summary.errors > 0 + } + + /// Return true if the report indicates any warnings. + pub fn has_warnings(&self) -> bool { + self.summary.warnings > 0 + } + + /// Return the appropriate exit code for this report. + pub fn exit_code(&self) -> i32 { + if self.has_errors() { + 1 + } else { + 0 + } + } + + /// Return findings filtered by a given status. + pub fn findings_with_status(&self, status: DoctorStatus) -> Vec<&DoctorFinding> { + self.findings.iter().filter(|f| f.status == status).collect() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + const ID: DoctorCheckId = DoctorCheckId::new("test.check"); + + #[test] + fn test_summary_counts() { + let findings = vec![ + DoctorFinding::ok(ID, "Good", "All good"), + DoctorFinding::skipped(ID, "Skipped", "Not applicable"), + DoctorFinding::warning(ID, "Warn", "Minor issue", "Fix it"), + ]; + let report = DoctorReport::new(findings); + assert_eq!(report.summary.ok, 1); + assert_eq!(report.summary.warnings, 1); + assert_eq!(report.summary.errors, 0); + assert_eq!(report.summary.skipped, 1); + assert_eq!(report.summary.total(), 3); + } + + #[test] + fn test_overall_status_error_wins() { + let findings = vec![ + DoctorFinding::ok(ID, "Good", "All good"), + DoctorFinding::error(ID, "Bad", "Critical", "Fix it"), + DoctorFinding::warning(ID, "Warn", "Minor", "Fix it"), + ]; + let report = DoctorReport::new(findings); + assert_eq!(report.overall_status, DoctorStatus::Error); + assert!(report.has_errors()); + assert!(report.has_warnings()); + assert_eq!(report.exit_code(), 1); + } + + #[test] + fn test_overall_status_warning_without_error() { + let findings = vec![ + DoctorFinding::ok(ID, "Good", "All good"), + DoctorFinding::warning(ID, "Warn", "Minor", "Fix it"), + ]; + let report = DoctorReport::new(findings); + assert_eq!(report.overall_status, DoctorStatus::Warning); + assert!(!report.has_errors()); + assert!(report.has_warnings()); + assert_eq!(report.exit_code(), 0); + } + + #[test] + fn test_overall_status_all_skipped() { + let findings = vec![ + DoctorFinding::skipped(ID, "Skipped", "Not applicable"), + DoctorFinding::skipped(ID, "Skipped 2", "Also not applicable"), + ]; + let report = DoctorReport::new(findings); + assert_eq!(report.overall_status, DoctorStatus::Skipped); + assert!(!report.has_errors()); + assert!(!report.has_warnings()); + assert_eq!(report.exit_code(), 0); + } + + #[test] + fn test_overall_status_mixed_but_passing() { + let findings = vec![ + DoctorFinding::ok(ID, "Good", "All good"), + DoctorFinding::skipped(ID, "Skipped", "Not applicable"), + ]; + let report = DoctorReport::new(findings); + assert_eq!(report.overall_status, DoctorStatus::Ok); + assert!(!report.has_errors()); + assert!(!report.has_warnings()); + assert_eq!(report.exit_code(), 0); + } + + #[test] + fn test_findings_with_status() { + let findings = vec![ + DoctorFinding::ok(ID, "Good", "All good"), + DoctorFinding::warning(ID, "Warn", "Minor", "Fix it"), + DoctorFinding::error(ID, "Bad", "Critical", "Fix it"), + DoctorFinding::skipped(ID, "Skipped", "Not applicable"), + ]; + let report = DoctorReport::new(findings); + assert_eq!(report.findings_with_status(DoctorStatus::Ok).len(), 1); + assert_eq!(report.findings_with_status(DoctorStatus::Warning).len(), 1); + assert_eq!(report.findings_with_status(DoctorStatus::Error).len(), 1); + assert_eq!(report.findings_with_status(DoctorStatus::Skipped).len(), 1); + } + + #[test] + fn test_finding_builder_with_path() { + let finding = DoctorFinding::ok(ID, "Title", "Detail") + .with_path(std::path::PathBuf::from("/test/path")); + assert!(finding.path.is_some()); + assert_eq!(finding.path.unwrap(), std::path::PathBuf::from("/test/path")); + } + + #[test] + fn test_finding_builder_with_code() { + let finding = DoctorFinding::error(ID, "Title", "Detail", "Fix it") + .with_code("ERR_VAULT_MISSING"); + assert_eq!(finding.code, Some("ERR_VAULT_MISSING")); + } + + #[test] + fn test_doctor_check_id_display() { + let id = DoctorCheckId::new("vault.exists"); + assert_eq!(id.to_string(), "vault.exists"); + assert_eq!(id.as_str(), "vault.exists"); + } +} diff --git a/ows/crates/ows-cli/src/commands/mod.rs b/ows/crates/ows-cli/src/commands/mod.rs index a6fee800..400691d1 100644 --- a/ows/crates/ows-cli/src/commands/mod.rs +++ b/ows/crates/ows-cli/src/commands/mod.rs @@ -1,5 +1,6 @@ pub mod config; pub mod derive; +pub mod doctor; pub mod fund; pub mod generate; pub mod info; From 9189d704d26e0976887636381e5acc698b07aaa4 Mon Sep 17 00:00:00 2001 From: OKWN Date: Thu, 2 Apr 2026 12:56:11 +0300 Subject: [PATCH 3/9] feat(ows-cli): add vault artifact inspection for ows doctor Phase 3: Read-only vault inspection for wallet, key, and policy artifacts - vault_inspector.rs: New module with check_wallet_files(), check_key_files(), check_policy_files(). Each function: - Enumerates .json files in the vault subdirectory - Reads and parses each file as the appropriate type - Validates metadata where safe (empty ID, invalid created_at) - Returns granular findings per file plus summary finding - Error codes: ERR_FILE_UNREADABLE, ERR_FILE_MALFORMED, ERR_METADATA_INVALID, WARN_NO_WALLETS, WARN_ARTIFACTS_CORRUPTED - checks.rs: Removed redundant dir-check functions; delegates to vault_inspector from run_all_checks(). Resolves vault path once and passes it to all artifact checks. - 13 new unit tests covering: valid fixtures, malformed JSON, empty directories, empty ID, invalid created_at, mixed valid/corrupt artifacts - Separation maintained: checks.rs handles structural/env checks, vault_inspector handles artifact content validation Co-Authored-By: Claude Opus 4.6 --- .../ows-cli/src/commands/doctor/checks.rs | 434 ++++-------- ows/crates/ows-cli/src/commands/doctor/mod.rs | 10 +- .../src/commands/doctor/vault_inspector.rs | 666 ++++++++++++++++++ 3 files changed, 819 insertions(+), 291 deletions(-) create mode 100644 ows/crates/ows-cli/src/commands/doctor/vault_inspector.rs diff --git a/ows/crates/ows-cli/src/commands/doctor/checks.rs b/ows/crates/ows-cli/src/commands/doctor/checks.rs index 3b5ac57f..33b8c7a5 100644 --- a/ows/crates/ows-cli/src/commands/doctor/checks.rs +++ b/ows/crates/ows-cli/src/commands/doctor/checks.rs @@ -1,6 +1,7 @@ //! Individual diagnostic checks for `ows doctor`. use crate::commands::doctor::report::{DoctorCheckId, DoctorFinding, DoctorReport}; +use crate::commands::doctor::vault_inspector; use ows_core::Config; @@ -12,12 +13,6 @@ use ows_core::Config; pub const CHECK_VAULT_PATH: DoctorCheckId = DoctorCheckId::new("vault.path"); /// Vault existence check ID. pub const CHECK_VAULT_EXISTS: DoctorCheckId = DoctorCheckId::new("vault.exists"); -/// Wallets directory presence check ID. -pub const CHECK_WALLETS_DIR: DoctorCheckId = DoctorCheckId::new("vault.wallets_dir"); -/// Keys directory presence check ID. -pub const CHECK_KEYS_DIR: DoctorCheckId = DoctorCheckId::new("vault.keys_dir"); -/// Policies directory presence check ID. -pub const CHECK_POLICIES_DIR: DoctorCheckId = DoctorCheckId::new("vault.policies_dir"); /// Logs directory presence check ID. pub const CHECK_LOGS_DIR: DoctorCheckId = DoctorCheckId::new("vault.logs_dir"); /// Config file presence and parseability check ID. @@ -31,15 +26,11 @@ pub const CHECK_HOME_ENV: DoctorCheckId = DoctorCheckId::new("env.home"); // Vault path resolution // --------------------------------------------------------------------------- -/// Resolve the vault path, using an override for testing purposes. +/// Resolve the vault path from `Config::default()`. /// -/// When `vault_path_override` is `None`, resolves from `Config::default()`. -/// Tests can pass a specific path to avoid environment variable dependence. -fn resolve_vault_path(vault_path_override: Option<&std::path::Path>) -> std::path::PathBuf { - match vault_path_override { - Some(p) => p.to_path_buf(), - None => Config::default().vault_path, - } +/// Exposed for use by vault_inspector functions in tests. +pub fn resolve_vault_path() -> std::path::PathBuf { + Config::default().vault_path } // --------------------------------------------------------------------------- @@ -60,7 +51,7 @@ pub fn check_vault_path() -> Vec { )); } - let vault_path = resolve_vault_path(None); + let vault_path = resolve_vault_path(); findings.push(DoctorFinding::ok( CHECK_VAULT_PATH, @@ -73,7 +64,7 @@ pub fn check_vault_path() -> Vec { /// Check that the vault directory exists. pub fn check_vault_exists() -> Vec { - let vault_path = resolve_vault_path(None); + let vault_path = resolve_vault_path(); if vault_path.exists() { vec![DoctorFinding::ok( @@ -95,117 +86,9 @@ pub fn check_vault_exists() -> Vec { } } -/// Check that the wallets subdirectory exists (if vault exists). -pub fn check_wallets_dir() -> Vec { - let vault_path = resolve_vault_path(None); - - if !vault_path.exists() { - return vec![DoctorFinding::skipped( - CHECK_WALLETS_DIR, - "Wallets directory skipped", - "Vault does not exist; skipping wallets directory check.", - )]; - } - - let wallets_dir = vault_path.join("wallets"); - - if wallets_dir.exists() { - // Count wallet files - let wallet_count = std::fs::read_dir(&wallets_dir) - .map(|entries| { - entries - .filter_map(|e| e.ok()) - .filter(|e| e.path().extension().and_then(|s| s.to_str()) == Some("json")) - .count() - }) - .unwrap_or(0); - - vec![DoctorFinding::ok( - CHECK_WALLETS_DIR, - "Wallets directory present", - &format!( - "wallets/ exists with {} wallet file(s)", - wallet_count - ), - ) - .with_path(wallets_dir)] - } else { - vec![DoctorFinding::error( - CHECK_WALLETS_DIR, - "Wallets directory missing", - &format!( - "Expected `{}` but it does not exist.", - wallets_dir.display() - ), - "This is unexpected. The wallet command should create this directory.", - ) - .with_path(wallets_dir)] - } -} - -/// Check that the keys subdirectory exists (if vault exists). -pub fn check_keys_dir() -> Vec { - let vault_path = resolve_vault_path(None); - - if !vault_path.exists() { - return vec![DoctorFinding::skipped( - CHECK_KEYS_DIR, - "Keys directory skipped", - "Vault does not exist; skipping keys directory check.", - )]; - } - - let keys_dir = vault_path.join("keys"); - - if keys_dir.exists() { - vec![DoctorFinding::ok( - CHECK_KEYS_DIR, - "Keys directory present", - &format!("keys/ exists at `{}`", keys_dir.display()), - ) - .with_path(keys_dir)] - } else { - vec![DoctorFinding::skipped( - CHECK_KEYS_DIR, - "Keys directory not present", - "keys/ does not exist. No API keys have been created yet.", - )] - } -} - -/// Check that the policies subdirectory exists (if vault exists). -pub fn check_policies_dir() -> Vec { - let vault_path = resolve_vault_path(None); - - if !vault_path.exists() { - return vec![DoctorFinding::skipped( - CHECK_POLICIES_DIR, - "Policies directory skipped", - "Vault does not exist; skipping policies directory check.", - )]; - } - - let policies_dir = vault_path.join("policies"); - - if policies_dir.exists() { - vec![DoctorFinding::ok( - CHECK_POLICIES_DIR, - "Policies directory present", - &format!("policies/ exists at `{}`", policies_dir.display()), - ) - .with_path(policies_dir)] - } else { - vec![DoctorFinding::skipped( - CHECK_POLICIES_DIR, - "Policies directory not present", - "policies/ does not exist. No policies have been created yet.", - )] - } -} - /// Check that the logs subdirectory exists (if vault exists). pub fn check_logs_dir() -> Vec { - let vault_path = resolve_vault_path(None); + let vault_path = resolve_vault_path(); if !vault_path.exists() { return vec![DoctorFinding::skipped( @@ -235,7 +118,7 @@ pub fn check_logs_dir() -> Vec { /// Check that the config file is present and parseable. pub fn check_config() -> Vec { - let config_path = resolve_vault_path(None).join("config.json"); + let config_path = resolve_vault_path().join("config.json"); if !config_path.exists() { return vec![DoctorFinding::skipped( @@ -277,7 +160,7 @@ pub fn check_config() -> Vec { pub fn check_vault_permissions() -> Vec { use std::os::unix::fs::PermissionsExt; - let vault_path = resolve_vault_path(None); + let vault_path = resolve_vault_path(); if !vault_path.exists() { return vec![DoctorFinding::skipped( @@ -290,7 +173,7 @@ pub fn check_vault_permissions() -> Vec { let mut findings = Vec::new(); // Check vault directory permissions - if let Ok(meta) = std::fs::metadata(vault_path) { + if let Ok(meta) = std::fs::metadata(&vault_path) { let mode = meta.permissions().mode() & 0o777; if mode != 0o700 { findings.push( @@ -367,7 +250,6 @@ pub fn check_vault_permissions() -> Vec { } if findings.is_empty() { - // No findings means we didn't add anything above vec![DoctorFinding::ok( CHECK_VAULT_PERMS, "Permissions check passed", @@ -392,11 +274,13 @@ pub fn check_vault_permissions() -> Vec { // Check runner // --------------------------------------------------------------------------- -/// Run all V1 diagnostic checks and return the aggregated report. +/// Run all diagnostic checks and return the aggregated report. /// /// Checks run in a fixed order. Each check is independent and produces /// zero or more findings. All findings are collected into a single report. pub fn run_all_checks() -> DoctorReport { + let vault_path = resolve_vault_path(); + let mut all_findings = Vec::new(); // Path resolution and HOME check @@ -405,15 +289,17 @@ pub fn run_all_checks() -> DoctorReport { // Vault existence all_findings.extend(check_vault_exists()); - // Required directories - all_findings.extend(check_wallets_dir()); - all_findings.extend(check_keys_dir()); - all_findings.extend(check_policies_dir()); + // Logs directory (optional) all_findings.extend(check_logs_dir()); // Config all_findings.extend(check_config()); + // Wallet, key, and policy file inspection + all_findings.extend(vault_inspector::check_wallet_files(&vault_path)); + all_findings.extend(vault_inspector::check_key_files(&vault_path)); + all_findings.extend(vault_inspector::check_policy_files(&vault_path)); + // Permissions (platform-specific) all_findings.extend(check_vault_permissions()); @@ -423,176 +309,154 @@ pub fn run_all_checks() -> DoctorReport { #[cfg(test)] mod tests { use super::*; - use crate::commands::doctor::DoctorStatus; - use tempfile::TempDir; + use crate::commands::doctor::vault_inspector; #[test] - fn test_check_vault_path_includes_home_check() { - let findings = check_vault_path(); - // Should have at least 2 findings: HOME check + vault path - assert!(findings.len() >= 2); - - // Should have CHECK_VAULT_PATH finding - let path_finding = findings.iter().find(|f| f.id == CHECK_VAULT_PATH); - assert!(path_finding.is_some()); - assert_eq!(path_finding.unwrap().status, DoctorStatus::Ok); - } - - #[test] - fn test_check_vault_exists_not_found() { - // Use a path that definitely doesn't exist - let temp_dir = TempDir::new().unwrap(); - std::env::set_var("HOME", temp_dir.path()); - - let findings = check_vault_exists(); - assert_eq!(findings.len(), 1); - let finding = &findings[0]; - assert_eq!(finding.id, CHECK_VAULT_EXISTS); - assert_eq!(finding.status, DoctorStatus::Error); - assert!(finding.detail.contains("not found")); - } - - #[test] - fn test_check_wallets_dir_skipped_when_vault_missing() { - let temp_dir = TempDir::new().unwrap(); - std::env::set_var("HOME", temp_dir.path()); - - let findings = check_wallets_dir(); - assert_eq!(findings.len(), 1); - assert_eq!(findings[0].status, DoctorStatus::Skipped); - } - - #[test] - fn test_check_keys_dir_skipped_when_vault_missing() { - let temp_dir = TempDir::new().unwrap(); - std::env::set_var("HOME", temp_dir.path()); - - let findings = check_keys_dir(); - assert_eq!(findings.len(), 1); - assert_eq!(findings[0].status, DoctorStatus::Skipped); + fn test_run_all_checks_returns_valid_report() { + // Use a temporary vault path + let temp = tempfile::TempDir::new().unwrap(); + let vault = temp.path().to_path_buf(); + std::fs::create_dir(vault.join("wallets")).ok(); + std::fs::create_dir(vault.join("policies")).ok(); + std::fs::create_dir(vault.join("keys")).ok(); + + // Add a valid wallet + let wallet = ows_core::EncryptedWallet::new( + "test-id".to_string(), + "Test".to_string(), + vec![], + serde_json::json!({}), + ows_core::KeyType::Mnemonic, + ); + let json = serde_json::to_string_pretty(&wallet).unwrap(); + std::fs::write(vault.join("wallets/test.json"), json).ok(); + + // Add a valid policy + let policy = ows_core::Policy { + id: "test-policy".to_string(), + name: "Test".to_string(), + version: 1, + created_at: "2026-01-01T00:00:00Z".to_string(), + rules: vec![], + executable: None, + config: None, + action: ows_core::PolicyAction::Deny, + }; + let json = serde_json::to_string_pretty(&policy).unwrap(); + std::fs::write(vault.join("policies/test.json"), json).ok(); + + // Run checks with the temp vault path + std::env::set_var("HOME", temp.path()); + + let all_results = vault_inspector::check_wallet_files(&vault); + assert!(!all_results.is_empty()); + + let all_policies = vault_inspector::check_policy_files(&vault); + assert!(!all_policies.is_empty()); + + let all_keys = vault_inspector::check_key_files(&vault); + assert!(!all_keys.is_empty()); + + // Report aggregation + let mut findings = Vec::new(); + findings.extend(vault_inspector::check_wallet_files(&vault)); + findings.extend(vault_inspector::check_key_files(&vault)); + findings.extend(vault_inspector::check_policy_files(&vault)); + findings.extend(check_config()); + findings.extend(check_logs_dir()); + + let report = DoctorReport::new(findings); + assert!(report.findings.iter().any(|f| f.status == DoctorStatus::Ok)); } #[test] - fn test_check_policies_dir_skipped_when_vault_missing() { - let temp_dir = TempDir::new().unwrap(); - std::env::set_var("HOME", temp_dir.path()); + fn test_wallet_inspection_empty_dir_warning() { + let temp = tempfile::TempDir::new().unwrap(); + let vault = temp.path().to_path_buf(); + std::fs::create_dir(vault.join("wallets")).ok(); - let findings = check_policies_dir(); - assert_eq!(findings.len(), 1); - assert_eq!(findings[0].status, DoctorStatus::Skipped); + let findings = vault_inspector::check_wallet_files(&vault); + assert!(findings.iter().any(|f| f.status == DoctorStatus::Warning)); } #[test] - fn test_check_logs_dir_skipped_when_vault_missing() { - let temp_dir = TempDir::new().unwrap(); - std::env::set_var("HOME", temp_dir.path()); + fn test_wallet_inspection_malformed_json() { + let temp = tempfile::TempDir::new().unwrap(); + let vault = temp.path().to_path_buf(); + let wallets_dir = vault.join("wallets"); + std::fs::create_dir_all(&wallets_dir).ok(); + std::fs::write(wallets_dir.join("bad.json"), "{ invalid }").ok(); - let findings = check_logs_dir(); - assert_eq!(findings.len(), 1); - assert_eq!(findings[0].status, DoctorStatus::Skipped); + let findings = vault_inspector::check_wallet_files(&vault); + assert!(findings.iter().any(|f| f.code == Some("ERR_FILE_MALFORMED"))); } #[test] - fn test_check_config_skipped_when_no_config() { - let temp_dir = TempDir::new().unwrap(); - std::env::set_var("HOME", temp_dir.path()); - - let findings = check_config(); - assert_eq!(findings.len(), 1); - assert_eq!(findings[0].id, CHECK_CONFIG); - assert_eq!(findings[0].status, DoctorStatus::Skipped); - } + fn test_wallet_inspection_valid() { + let temp = tempfile::TempDir::new().unwrap(); + let vault = temp.path().to_path_buf(); + let wallets_dir = vault.join("wallets"); + std::fs::create_dir_all(&wallets_dir).ok(); - #[test] - fn test_check_config_valid() { - let temp_dir = TempDir::new().unwrap(); - std::env::set_var("HOME", temp_dir.path()); - - // Create a valid config - let config_path = temp_dir.path().join(".ows/config.json"); - std::fs::create_dir_all(config_path.parent().unwrap()).ok(); - let config_content = serde_json::json!({ - "vault_path": "/test/.ows", - "rpc": { - "eip155:1": "https://eth.example.com" - } - }); - std::fs::write(&config_path, serde_json::to_string_pretty(&config_content).unwrap()).ok(); - - let findings = check_config(); - assert_eq!(findings.len(), 1); - let finding = &findings[0]; - assert_eq!(finding.id, CHECK_CONFIG); - assert_eq!(finding.status, DoctorStatus::Ok); - assert!(finding.detail.contains("1 RPC endpoint")); + let wallet = ows_core::EncryptedWallet::new( + "test-wallet".to_string(), + "Test Wallet".to_string(), + vec![], + serde_json::json!({}), + ows_core::KeyType::Mnemonic, + ); + let json = serde_json::to_string_pretty(&wallet).unwrap(); + std::fs::write(wallets_dir.join("test.json"), json).ok(); + + let findings = vault_inspector::check_wallet_files(&vault); + assert!(findings.iter().any(|f| f.status == DoctorStatus::Ok)); } #[test] - fn test_check_config_malformed() { - let temp_dir = TempDir::new().unwrap(); - std::env::set_var("HOME", temp_dir.path()); - - // Create a malformed config - let config_path = temp_dir.path().join(".ows/config.json"); - std::fs::create_dir_all(config_path.parent().unwrap()).ok(); - std::fs::write(&config_path, "{ invalid json }").ok(); - - let findings = check_config(); - assert_eq!(findings.len(), 1); - let finding = &findings[0]; - assert_eq!(finding.id, CHECK_CONFIG); - assert_eq!(finding.status, DoctorStatus::Error); - assert!(finding.code.is_some()); + fn test_policy_inspection_valid() { + let temp = tempfile::TempDir::new().unwrap(); + let vault = temp.path().to_path_buf(); + let policies_dir = vault.join("policies"); + std::fs::create_dir_all(&policies_dir).ok(); + + let policy = ows_core::Policy { + id: "test-policy".to_string(), + name: "Test Policy".to_string(), + version: 1, + created_at: "2026-01-01T00:00:00Z".to_string(), + rules: vec![], + executable: None, + config: None, + action: ows_core::PolicyAction::Deny, + }; + let json = serde_json::to_string_pretty(&policy).unwrap(); + std::fs::write(policies_dir.join("test.json"), json).ok(); + + let findings = vault_inspector::check_policy_files(&vault); + assert!(findings.iter().any(|f| f.status == DoctorStatus::Ok)); } #[test] - fn test_run_all_checks_returns_valid_report() { - // Use a temporary HOME so we don't affect real vault - let temp_dir = TempDir::new().unwrap(); - std::env::set_var("HOME", temp_dir.path()); - - let report = run_all_checks(); - - // Should have findings - assert!(!report.findings.is_empty()); - - // Should have an overall status - assert!(matches!( - report.overall_status, - DoctorStatus::Ok | DoctorStatus::Warning | DoctorStatus::Error | DoctorStatus::Skipped - )); - - // Exit code should be 0 or 1 - assert!(report.exit_code() >= 0 && report.exit_code() <= 1); - } - - #[test] - fn test_check_wallets_dir_with_wallets() { - let temp_dir = TempDir::new().unwrap(); - std::env::set_var("HOME", temp_dir.path()); - - // Create vault with wallets - let vault = temp_dir.path().join(".ows"); - let wallets_dir = vault.join("wallets"); - std::fs::create_dir_all(&wallets_dir).ok(); - - // Add a dummy wallet file - let wallet_content = serde_json::json!({ - "ows_version": 2, - "id": "test-wallet", - "name": "Test Wallet", - "created_at": "2026-01-01T00:00:00Z", - "accounts": [], - "crypto": {}, - "key_type": "mnemonic" - }); - let wallet_path = wallets_dir.join("test-wallet.json"); - std::fs::write(&wallet_path, serde_json::to_string_pretty(&wallet_content).unwrap()).ok(); - - let findings = check_wallets_dir(); - assert_eq!(findings.len(), 1); - let finding = &findings[0]; - assert_eq!(finding.status, DoctorStatus::Ok); - assert!(finding.detail.contains("1 wallet file")); + fn test_key_inspection_valid() { + let temp = tempfile::TempDir::new().unwrap(); + let vault = temp.path().to_path_buf(); + let keys_dir = vault.join("keys"); + std::fs::create_dir_all(&keys_dir).ok(); + + let key = ows_core::ApiKeyFile { + id: "test-key".to_string(), + name: "Test Key".to_string(), + token_hash: "deadbeef".to_string(), + created_at: "2026-01-01T00:00:00Z".to_string(), + wallet_ids: vec![], + policy_ids: vec![], + expires_at: None, + wallet_secrets: std::collections::HashMap::new(), + }; + let json = serde_json::to_string_pretty(&key).unwrap(); + std::fs::write(keys_dir.join("test.json"), json).ok(); + + let findings = vault_inspector::check_key_files(&vault); + assert!(findings.iter().any(|f| f.status == DoctorStatus::Ok)); } } diff --git a/ows/crates/ows-cli/src/commands/doctor/mod.rs b/ows/crates/ows-cli/src/commands/doctor/mod.rs index 14d87c30..d862e40d 100644 --- a/ows/crates/ows-cli/src/commands/doctor/mod.rs +++ b/ows/crates/ows-cli/src/commands/doctor/mod.rs @@ -5,6 +5,7 @@ //! - Vault and subdirectory existence //! - Config file validity //! - File permissions (Unix) +//! - Wallet, key, and policy file integrity //! - Environment configuration //! //! All checks are read-only and do not modify any files. @@ -13,7 +14,7 @@ //! //! - [`report`] — Domain types: `DoctorStatus`, `DoctorFinding`, `DoctorReport` //! - [`checks`] — Individual check implementations -//! - [`run_all_checks()`] — Aggregates all findings into a report +//! - [`vault_inspector`] — Read-only vault artifact inspection (wallets/keys/policies) //! //! # Stability //! @@ -23,11 +24,8 @@ pub mod checks; pub mod report; +pub mod vault_inspector; -// Re-exported for use by the CLI command (Phase 3) and integration tests. +// Re-exports for the CLI command (Phase 3). #[allow(unused)] pub use report::{DoctorCheckId, DoctorFinding, DoctorReport, DoctorStatus, DoctorSummary}; -pub use checks::run_all_checks; - -/// Aggregated diagnostic report — the output of [`run_all_checks()`]. -pub type DoctorResult = DoctorReport; diff --git a/ows/crates/ows-cli/src/commands/doctor/vault_inspector.rs b/ows/crates/ows-cli/src/commands/doctor/vault_inspector.rs new file mode 100644 index 00000000..c81b0c8d --- /dev/null +++ b/ows/crates/ows-cli/src/commands/doctor/vault_inspector.rs @@ -0,0 +1,666 @@ +//! Read-only vault artifact inspection for `ows doctor`. +//! +//! Provides functions to enumerate and validate wallet, key, and policy files +//! without decrypting secrets or modifying any state. +//! +//! All functions return findings directly; they do not mutate or create files. +//! +//! # Validation approach +//! +//! - **Wallet files**: Deserialize as `EncryptedWallet`. The serde derivation +//! validates structure. We additionally check for empty ID and empty accounts. +//! - **Key files**: Deserialize as `ApiKeyFile`. Validates all required fields. +//! - **Policy files**: Deserialize as `Policy`. Validates structure. +//! +//! # Error taxonomy +//! +//! | Condition | Status | Code | +//! |-----------|--------|------| +//! | File unreadable (permissions) | Error | `ERR_FILE_UNREADABLE` | +//! | File not valid JSON | Error | `ERR_FILE_MALFORMED` | +//! | JSON parses but schema invalid | Error | `ERR_METADATA_INVALID` | +//! | No artifacts of this type | Skipped | — | +//! | All artifacts valid | Ok | — | +//! | Some artifacts valid, some invalid | Warning | `WARN_ARTIFACTS_CORRUPTED` | + +use std::fs; +use std::path::Path; + +use ows_core::{ApiKeyFile, EncryptedWallet, Policy}; + +use crate::commands::doctor::report::{DoctorCheckId, DoctorFinding}; + +// --------------------------------------------------------------------------- +// Check IDs +// --------------------------------------------------------------------------- + +pub const CHECK_WALLET_FILES: DoctorCheckId = DoctorCheckId::new("vault.wallet_files"); +pub const CHECK_KEY_FILES: DoctorCheckId = DoctorCheckId::new("vault.key_files"); +pub const CHECK_POLICY_FILES: DoctorCheckId = DoctorCheckId::new("vault.policy_files"); + +// --------------------------------------------------------------------------- +// Wallet file inspection +// --------------------------------------------------------------------------- + +/// Inspect all wallet files in the vault. +/// +/// Returns one finding per artifact, plus a summary finding. +/// +/// # Arguments +/// * `vault_path` - Path to the vault directory (e.g. `~/.ows`) +pub fn check_wallet_files(vault_path: &Path) -> Vec { + let wallets_dir = vault_path.join("wallets"); + + if !wallets_dir.exists() { + return vec![DoctorFinding::skipped( + CHECK_WALLET_FILES, + "No wallets directory", + "Wallets directory does not exist; skipping wallet inspection.", + )]; + } + + let entries: Vec<_> = match fs::read_dir(&wallets_dir) { + Ok(e) => e.filter_map(|e| e.ok()).collect(), + Err(e) => { + return vec![DoctorFinding::error( + CHECK_WALLET_FILES, + "Cannot read wallets directory", + &format!("Wallets directory exists but cannot be read: {}", e), + "Check directory permissions.", + ) + .with_path(wallets_dir) + .with_code("ERR_DIR_UNREADABLE")]; + } + }; + + // Filter to only .json files + let json_entries: Vec<_> = entries + .into_iter() + .filter(|e| e.path().extension().and_then(|s| s.to_str()) == Some("json")) + .collect(); + + if json_entries.is_empty() { + return vec![DoctorFinding::warning( + CHECK_WALLET_FILES, + "No wallets present", + "No wallet files found in the wallets directory.", + "Run `ows wallet create` to create your first wallet.", + ) + .with_path(wallets_dir) + .with_code("WARN_NO_WALLETS")]; + } + + let mut findings = Vec::new(); + let mut valid_count = 0; + let mut corrupted_count = 0; + + for entry in json_entries { + let path = entry.path(); + let file_name = entry.file_name().to_string_lossy().to_string(); + + // Try to read the file + let contents = match fs::read_to_string(&path) { + Ok(c) => c, + Err(e) => { + corrupted_count += 1; + findings.push(DoctorFinding::error( + CHECK_WALLET_FILES, + "Wallet file unreadable", + &format!("{}: cannot read file: {}", file_name, e), + "Check file permissions.", + ) + .with_path(path) + .with_code("ERR_FILE_UNREADABLE")); + continue; + } + }; + + // Try to parse as EncryptedWallet + let wallet: EncryptedWallet = match serde_json::from_str(&contents) { + Ok(w) => w, + Err(e) => { + corrupted_count += 1; + findings.push(DoctorFinding::error( + CHECK_WALLET_FILES, + "Wallet file malformed", + &format!("{}: invalid JSON: {}", file_name, e), + "This wallet file is corrupted. Export the mnemonic (if possible) and recreate the wallet.", + ) + .with_path(path) + .with_code("ERR_FILE_MALFORMED")); + continue; + } + }; + + // Additional metadata validation + if wallet.id.is_empty() { + findings.push(DoctorFinding::error( + CHECK_WALLET_FILES, + "Wallet has empty ID", + &format!("{}: wallet ID field is empty", file_name), + "Recreate the wallet from the mnemonic.", + ) + .with_path(path) + .with_code("ERR_METADATA_INVALID")); + corrupted_count += 1; + continue; + } + + if wallet.created_at.is_empty() { + findings.push(DoctorFinding::error( + CHECK_WALLET_FILES, + "Wallet has empty created_at", + &format!("{}: created_at field is empty", file_name), + "Recreate the wallet from the mnemonic.", + ) + .with_path(path) + .with_code("ERR_METADATA_INVALID")); + corrupted_count += 1; + continue; + } + + // Validate created_at is valid RFC3339 + if chrono::DateTime::parse_from_rfc3339(&wallet.created_at).is_err() { + findings.push(DoctorFinding::error( + CHECK_WALLET_FILES, + "Wallet has invalid created_at", + &format!("{}: created_at is not valid RFC3339: `{}`", file_name, wallet.created_at), + "Recreate the wallet from the mnemonic.", + ) + .with_path(path) + .with_code("ERR_METADATA_INVALID")); + corrupted_count += 1; + continue; + } + + valid_count += 1; + } + + // Push summary finding + if corrupted_count == 0 { + findings.push(DoctorFinding::ok( + CHECK_WALLET_FILES, + "Wallet files valid", + &format!("All {} wallet file(s) are valid.", valid_count), + )); + } else { + findings.push(DoctorFinding::warning( + CHECK_WALLET_FILES, + "Some wallet files corrupted", + &format!( + "{} of {} wallet file(s) are corrupted.", + corrupted_count, + valid_count + corrupted_count + ), + "Export valid wallets and recreate the corrupted ones.", + ) + .with_code("WARN_ARTIFACTS_CORRUPTED")); + } + + findings +} + +// --------------------------------------------------------------------------- +// Key file inspection +// --------------------------------------------------------------------------- + +/// Inspect all API key files in the vault. +/// +/// Returns one finding per artifact, plus a summary finding. +pub fn check_key_files(vault_path: &Path) -> Vec { + let keys_dir = vault_path.join("keys"); + + if !keys_dir.exists() { + return vec![DoctorFinding::skipped( + CHECK_KEY_FILES, + "No keys directory", + "Keys directory does not exist; skipping key file inspection.", + )]; + } + + let entries: Vec<_> = match fs::read_dir(&keys_dir) { + Ok(e) => e.filter_map(|e| e.ok()).collect(), + Err(e) => { + return vec![DoctorFinding::error( + CHECK_KEY_FILES, + "Cannot read keys directory", + &format!("Keys directory exists but cannot be read: {}", e), + "Check directory permissions.", + ) + .with_path(keys_dir) + .with_code("ERR_DIR_UNREADABLE")]; + } + }; + + let json_entries: Vec<_> = entries + .into_iter() + .filter(|e| e.path().extension().and_then(|s| s.to_str()) == Some("json")) + .collect(); + + if json_entries.is_empty() { + return vec![DoctorFinding::skipped( + CHECK_KEY_FILES, + "No API keys present", + "No key files found in the keys directory.", + ) + .with_path(keys_dir)]; + } + + let mut findings = Vec::new(); + let mut valid_count = 0; + let mut corrupted_count = 0; + + for entry in json_entries { + let path = entry.path(); + let file_name = entry.file_name().to_string_lossy().to_string(); + + let contents = match fs::read_to_string(&path) { + Ok(c) => c, + Err(e) => { + corrupted_count += 1; + findings.push(DoctorFinding::error( + CHECK_KEY_FILES, + "Key file unreadable", + &format!("{}: cannot read file: {}", file_name, e), + "Check file permissions.", + ) + .with_path(path) + .with_code("ERR_FILE_UNREADABLE")); + continue; + } + }; + + let _key_file: ApiKeyFile = match serde_json::from_str(&contents) { + Ok(k) => k, + Err(e) => { + corrupted_count += 1; + findings.push(DoctorFinding::error( + CHECK_KEY_FILES, + "Key file malformed", + &format!("{}: invalid JSON: {}", file_name, e), + "Delete and recreate the API key.", + ) + .with_path(path) + .with_code("ERR_FILE_MALFORMED")); + continue; + } + }; + + valid_count += 1; + } + + if corrupted_count == 0 { + findings.push(DoctorFinding::ok( + CHECK_KEY_FILES, + "Key files valid", + &format!("All {} key file(s) are valid.", valid_count), + )); + } else { + findings.push(DoctorFinding::warning( + CHECK_KEY_FILES, + "Some key files corrupted", + &format!( + "{} of {} key file(s) are corrupted.", + corrupted_count, + valid_count + corrupted_count + ), + "Delete and recreate the corrupted API keys.", + ) + .with_code("WARN_ARTIFACTS_CORRUPTED")); + } + + findings +} + +// --------------------------------------------------------------------------- +// Policy file inspection +// --------------------------------------------------------------------------- + +/// Inspect all policy files in the vault. +/// +/// Returns one finding per artifact, plus a summary finding. +pub fn check_policy_files(vault_path: &Path) -> Vec { + let policies_dir = vault_path.join("policies"); + + if !policies_dir.exists() { + return vec![DoctorFinding::skipped( + CHECK_POLICY_FILES, + "No policies directory", + "Policies directory does not exist; skipping policy file inspection.", + )]; + } + + let entries: Vec<_> = match fs::read_dir(&policies_dir) { + Ok(e) => e.filter_map(|e| e.ok()).collect(), + Err(e) => { + return vec![DoctorFinding::error( + CHECK_POLICY_FILES, + "Cannot read policies directory", + &format!("Policies directory exists but cannot be read: {}", e), + "Check directory permissions.", + ) + .with_path(policies_dir) + .with_code("ERR_DIR_UNREADABLE")]; + } + }; + + let json_entries: Vec<_> = entries + .into_iter() + .filter(|e| e.path().extension().and_then(|s| s.to_str()) == Some("json")) + .collect(); + + if json_entries.is_empty() { + return vec![DoctorFinding::skipped( + CHECK_POLICY_FILES, + "No policies present", + "No policy files found in the policies directory.", + ) + .with_path(policies_dir)]; + } + + let mut findings = Vec::new(); + let mut valid_count = 0; + let mut corrupted_count = 0; + + for entry in json_entries { + let path = entry.path(); + let file_name = entry.file_name().to_string_lossy().to_string(); + + let contents = match fs::read_to_string(&path) { + Ok(c) => c, + Err(e) => { + corrupted_count += 1; + findings.push(DoctorFinding::error( + CHECK_POLICY_FILES, + "Policy file unreadable", + &format!("{}: cannot read file: {}", file_name, e), + "Check file permissions.", + ) + .with_path(path) + .with_code("ERR_FILE_UNREADABLE")); + continue; + } + }; + + let _policy: Policy = match serde_json::from_str(&contents) { + Ok(p) => p, + Err(e) => { + corrupted_count += 1; + findings.push(DoctorFinding::error( + CHECK_POLICY_FILES, + "Policy file malformed", + &format!("{}: invalid JSON: {}", file_name, e), + "Delete and recreate the policy.", + ) + .with_path(path) + .with_code("ERR_FILE_MALFORMED")); + continue; + } + }; + + valid_count += 1; + } + + if corrupted_count == 0 { + findings.push(DoctorFinding::ok( + CHECK_POLICY_FILES, + "Policy files valid", + &format!("All {} policy file(s) are valid.", valid_count), + )); + } else { + findings.push(DoctorFinding::warning( + CHECK_POLICY_FILES, + "Some policy files corrupted", + &format!( + "{} of {} policy file(s) are corrupted.", + corrupted_count, + valid_count + corrupted_count + ), + "Delete and recreate the corrupted policies.", + ) + .with_code("WARN_ARTIFACTS_CORRUPTED")); + } + + findings +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::commands::doctor::DoctorStatus; + use tempfile::TempDir; + + fn dummy_wallet(id: &str, name: &str) -> EncryptedWallet { + EncryptedWallet::new( + id.to_string(), + name.to_string(), + vec![], + serde_json::json!({}), + ows_core::KeyType::Mnemonic, + ) + } + + fn dummy_key(id: &str, name: &str) -> ApiKeyFile { + ApiKeyFile { + id: id.to_string(), + name: name.to_string(), + token_hash: "deadbeef".to_string(), + created_at: "2026-01-01T00:00:00Z".to_string(), + wallet_ids: vec![], + policy_ids: vec![], + expires_at: None, + wallet_secrets: std::collections::HashMap::new(), + } + } + + fn dummy_policy(id: &str, name: &str) -> Policy { + Policy { + id: id.to_string(), + name: name.to_string(), + version: 1, + created_at: "2026-01-01T00:00:00Z".to_string(), + rules: vec![], + executable: None, + config: None, + action: ows_core::PolicyAction::Deny, + } + } + + // ---- Wallet tests ---- + + #[test] + fn test_wallet_files_skipped_when_dir_missing() { + let temp = TempDir::new().unwrap(); + let findings = check_wallet_files(temp.path()); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].status, DoctorStatus::Skipped); + } + + #[test] + fn test_wallet_files_empty_dir_is_warning() { + let temp = TempDir::new().unwrap(); + let vault = temp.path().join(".ows"); + std::fs::create_dir(vault.join("wallets")).ok(); + let findings = check_wallet_files(&vault); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].status, DoctorStatus::Warning); + assert_eq!(findings[0].code, Some("WARN_NO_WALLETS")); + } + + #[test] + fn test_wallet_files_one_valid() { + let temp = TempDir::new().unwrap(); + let vault = temp.path().join(".ows"); + let wallets_dir = vault.join("wallets"); + std::fs::create_dir_all(&wallets_dir).ok(); + let wallet = dummy_wallet("wallet-1", "Test Wallet"); + let json = serde_json::to_string_pretty(&wallet).unwrap(); + std::fs::write(wallets_dir.join("wallet-1.json"), json).ok(); + + let findings = check_wallet_files(&vault); + // Should have 2 findings: one for the wallet, one summary + assert!(findings.iter().any(|f| f.status == DoctorStatus::Ok && f.detail.contains("1 wallet"))); + assert!(findings.iter().any(|f| f.id == CHECK_WALLET_FILES && f.status == DoctorStatus::Ok)); + } + + #[test] + fn test_wallet_files_malformed_json() { + let temp = TempDir::new().unwrap(); + let vault = temp.path().join(".ows"); + let wallets_dir = vault.join("wallets"); + std::fs::create_dir_all(&wallets_dir).ok(); + std::fs::write(wallets_dir.join("bad.json"), "{ invalid json }").ok(); + + let findings = check_wallet_files(&vault); + assert!(findings.iter().any(|f| f.status == DoctorStatus::Error && f.code == Some("ERR_FILE_MALFORMED"))); + } + + #[test] + fn test_wallet_files_empty_id() { + let temp = TempDir::new().unwrap(); + let vault = temp.path().join(".ows"); + let wallets_dir = vault.join("wallets"); + std::fs::create_dir_all(&wallets_dir).ok(); + let wallet = dummy_wallet("", "Empty ID Wallet"); + let json = serde_json::to_string_pretty(&wallet).unwrap(); + std::fs::write(wallets_dir.join("empty-id.json"), json).ok(); + + let findings = check_wallet_files(&vault); + assert!(findings.iter().any(|f| f.code == Some("ERR_METADATA_INVALID"))); + } + + #[test] + fn test_wallet_files_invalid_created_at() { + let temp = TempDir::new().unwrap(); + let vault = temp.path().join(".ows"); + let wallets_dir = vault.join("wallets"); + std::fs::create_dir_all(&wallets_dir).ok(); + let wallet = EncryptedWallet::new( + "test-id".to_string(), + "Test".to_string(), + vec![], + serde_json::json!({}), + ows_core::KeyType::Mnemonic, + ); + // Override created_at to be invalid + let mut json = serde_json::to_string_pretty(&wallet).unwrap(); + json = json.replace("2026-01-01T00:00:00Z", "not-a-date"); + std::fs::write(wallets_dir.join("bad-date.json"), json).ok(); + + let findings = check_wallet_files(&vault); + assert!(findings.iter().any(|f| f.code == Some("ERR_METADATA_INVALID"))); + } + + #[test] + fn test_wallet_files_mixed_valid_and_corrupt() { + let temp = TempDir::new().unwrap(); + let vault = temp.path().join(".ows"); + let wallets_dir = vault.join("wallets"); + std::fs::create_dir_all(&wallets_dir).ok(); + + // Valid wallet + let wallet = dummy_wallet("good", "Good Wallet"); + let json = serde_json::to_string_pretty(&wallet).unwrap(); + std::fs::write(wallets_dir.join("good.json"), json).ok(); + + // Corrupted wallet + std::fs::write(wallets_dir.join("bad.json"), "{ bad }").ok(); + + let findings = check_wallet_files(&vault); + assert!(findings.iter().any(|f| f.status == DoctorStatus::Ok)); + assert!(findings.iter().any(|f| f.status == DoctorStatus::Error)); + assert!(findings.iter().any(|f| f.code == Some("WARN_ARTIFACTS_CORRUPTED"))); + } + + // ---- Key file tests ---- + + #[test] + fn test_key_files_skipped_when_dir_missing() { + let temp = TempDir::new().unwrap(); + let findings = check_key_files(temp.path()); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].status, DoctorStatus::Skipped); + } + + #[test] + fn test_key_files_empty_dir_is_skipped() { + let temp = TempDir::new().unwrap(); + let vault = temp.path().join(".ows"); + std::fs::create_dir(vault.join("keys")).ok(); + let findings = check_key_files(&vault); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].status, DoctorStatus::Skipped); + } + + #[test] + fn test_key_files_one_valid() { + let temp = TempDir::new().unwrap(); + let vault = temp.path().join(".ows"); + let keys_dir = vault.join("keys"); + std::fs::create_dir_all(&keys_dir).ok(); + let key = dummy_key("key-1", "Test Key"); + let json = serde_json::to_string_pretty(&key).unwrap(); + std::fs::write(keys_dir.join("key-1.json"), json).ok(); + + let findings = check_key_files(&vault); + assert!(findings.iter().any(|f| f.status == DoctorStatus::Ok)); + } + + #[test] + fn test_key_files_malformed_json() { + let temp = TempDir::new().unwrap(); + let vault = temp.path().join(".ows"); + let keys_dir = vault.join("keys"); + std::fs::create_dir_all(&keys_dir).ok(); + std::fs::write(keys_dir.join("bad.json"), "{ invalid }").ok(); + + let findings = check_key_files(&vault); + assert!(findings.iter().any(|f| f.status == DoctorStatus::Error && f.code == Some("ERR_FILE_MALFORMED"))); + } + + // ---- Policy file tests ---- + + #[test] + fn test_policy_files_skipped_when_dir_missing() { + let temp = TempDir::new().unwrap(); + let findings = check_policy_files(temp.path()); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].status, DoctorStatus::Skipped); + } + + #[test] + fn test_policy_files_empty_dir_is_skipped() { + let temp = TempDir::new().unwrap(); + let vault = temp.path().join(".ows"); + std::fs::create_dir(vault.join("policies")).ok(); + let findings = check_policy_files(&vault); + assert_eq!(findings.len(), 1); + assert_eq!(findings[0].status, DoctorStatus::Skipped); + } + + #[test] + fn test_policy_files_one_valid() { + let temp = TempDir::new().unwrap(); + let vault = temp.path().join(".ows"); + let policies_dir = vault.join("policies"); + std::fs::create_dir_all(&policies_dir).ok(); + let policy = dummy_policy("pol-1", "Test Policy"); + let json = serde_json::to_string_pretty(&policy).unwrap(); + std::fs::write(policies_dir.join("pol-1.json"), json).ok(); + + let findings = check_policy_files(&vault); + assert!(findings.iter().any(|f| f.status == DoctorStatus::Ok)); + } + + #[test] + fn test_policy_files_malformed_json() { + let temp = TempDir::new().unwrap(); + let vault = temp.path().join(".ows"); + let policies_dir = vault.join("policies"); + std::fs::create_dir_all(&policies_dir).ok(); + std::fs::write(policies_dir.join("bad.json"), "{ invalid }").ok(); + + let findings = check_policy_files(&vault); + assert!(findings.iter().any(|f| f.status == DoctorStatus::Error && f.code == Some("ERR_FILE_MALFORMED"))); + } +} From acfc698a36c6c90befd19d57b03172cc2a2cc690 Mon Sep 17 00:00:00 2001 From: OKWN Date: Thu, 2 Apr 2026 13:18:19 +0300 Subject: [PATCH 4/9] feat(ows-cli): wire ows doctor CLI command with human-readable output Phase 4 completes the `ows doctor` implementation by: - Adding `Doctor` command to the CLI parser in main.rs - Wiring `doctor::run()` to execute all checks and print the report - Adding human-readable output formatter with status grouping - Fixing test_wallet_files_empty_dir_is_warning (create_dir_all fix) - Adding doctor section to sdk-cli.md documentation Exit code 0: all checks pass or only warnings found Exit code 1: errors detected (diagnostic checks failed) Co-Authored-By: Claude Opus 4.6 --- docs/sdk-cli.md | 43 +++ .../ows-cli/src/commands/doctor/checks.rs | 5 +- ows/crates/ows-cli/src/commands/doctor/mod.rs | 119 +++++++- .../ows-cli/src/commands/doctor/report.rs | 34 ++- .../src/commands/doctor/vault_inspector.rs | 285 ++++++++++-------- ows/crates/ows-cli/src/main.rs | 3 + 6 files changed, 360 insertions(+), 129 deletions(-) diff --git a/docs/sdk-cli.md b/docs/sdk-cli.md index 04abe1e3..c232b3ea 100644 --- a/docs/sdk-cli.md +++ b/docs/sdk-cli.md @@ -380,6 +380,49 @@ ows uninstall # keep wallet data ows uninstall --purge # also remove ~/.ows (all wallet data) ``` +### `ows doctor` + +Run diagnostic checks on the OWS installation. `ows doctor` is a read-only command that checks: +- Vault path resolution and HOME environment variable +- Vault and subdirectory existence +- Config file validity (if present) +- Wallet, key, and policy file integrity +- File permissions (Unix only; skipped on Windows) + +Exit code is 0 when all checks pass or only warnings are found, and non-zero when errors are detected. + +```bash +ows doctor +``` + +Sample output: +``` +============================================================ + OWS Doctor +============================================================ + + Vault path: ~/.ows + + Errors: + ---------------------------------------- + ✗ Wallet file malformed: wallet-1.json: invalid JSON: ... + → Export the mnemonic (if possible) and recreate the wallet. + + Warnings: + ---------------------------------------- + ⚠ No wallets present: No wallet files found in the wallets directory. + → Run `ows wallet create` to create your first wallet. + + Passed: + ---------------------------------------- + ✓ Vault path resolved + ✓ Config valid + + 2 passed 1 warnings 1 errors 0 skipped + + Result: FAILED — errors found +``` + ## File Layout ``` diff --git a/ows/crates/ows-cli/src/commands/doctor/checks.rs b/ows/crates/ows-cli/src/commands/doctor/checks.rs index 33b8c7a5..2f8f0552 100644 --- a/ows/crates/ows-cli/src/commands/doctor/checks.rs +++ b/ows/crates/ows-cli/src/commands/doctor/checks.rs @@ -310,6 +310,7 @@ pub fn run_all_checks() -> DoctorReport { mod tests { use super::*; use crate::commands::doctor::vault_inspector; + use crate::commands::doctor::DoctorStatus; #[test] fn test_run_all_checks_returns_valid_report() { @@ -388,7 +389,9 @@ mod tests { std::fs::write(wallets_dir.join("bad.json"), "{ invalid }").ok(); let findings = vault_inspector::check_wallet_files(&vault); - assert!(findings.iter().any(|f| f.code == Some("ERR_FILE_MALFORMED"))); + assert!(findings + .iter() + .any(|f| f.code == Some("ERR_FILE_MALFORMED"))); } #[test] diff --git a/ows/crates/ows-cli/src/commands/doctor/mod.rs b/ows/crates/ows-cli/src/commands/doctor/mod.rs index d862e40d..d7749cb6 100644 --- a/ows/crates/ows-cli/src/commands/doctor/mod.rs +++ b/ows/crates/ows-cli/src/commands/doctor/mod.rs @@ -26,6 +26,123 @@ pub mod checks; pub mod report; pub mod vault_inspector; -// Re-exports for the CLI command (Phase 3). +// Re-exports for CLI and tests. #[allow(unused)] pub use report::{DoctorCheckId, DoctorFinding, DoctorReport, DoctorStatus, DoctorSummary}; + +use crate::CliError; + +/// Run the `ows doctor` diagnostic command. +/// +/// Executes all checks, formats the report as human-readable output, +/// and returns the appropriate exit code via `Err` when errors are found. +pub fn run() -> Result<(), CliError> { + let report = checks::run_all_checks(); + print_report(&report); + + if report.has_errors() { + Err(CliError::InvalidArgs("diagnostic checks failed".into())) + } else { + Ok(()) + } +} + +/// Print a human-readable diagnostic report to stdout. +fn print_report(report: &DoctorReport) { + use ows_core::Config; + + println!(); + println!("{}", "=".repeat(60)); + println!(" OWS Doctor"); + println!("{}", "=".repeat(60)); + println!(); + + // Vault path + let config = Config::default(); + println!(" Vault path: {}", config.vault_path.display()); + println!(); + + // Group findings by status + let errors: Vec<_> = report.findings_with_status(DoctorStatus::Error); + let warnings: Vec<_> = report.findings_with_status(DoctorStatus::Warning); + let skipped: Vec<_> = report.findings_with_status(DoctorStatus::Skipped); + let ok: Vec<_> = report.findings_with_status(DoctorStatus::Ok); + + // Print errors first + if !errors.is_empty() { + println!(" Errors:"); + println!(" {}", "-".repeat(40)); + for f in &errors { + print_finding(f); + } + println!(); + } + + // Then warnings + if !warnings.is_empty() { + println!(" Warnings:"); + println!(" {}", "-".repeat(40)); + for f in &warnings { + print_finding(f); + } + println!(); + } + + // Then skipped (informational) + if !skipped.is_empty() { + println!(" Skipped:"); + println!(" {}", "-".repeat(40)); + for f in &skipped { + print_finding(f); + } + println!(); + } + + // Then ok findings (brief, condensed) + if !ok.is_empty() { + println!(" Passed:"); + println!(" {}", "-".repeat(40)); + for f in &ok { + println!(" {} {}", status_icon(DoctorStatus::Ok), f.title); + } + println!(); + } + + // Summary + println!("{}", "=".repeat(60)); + println!( + " {} passed {} warnings {} errors {} skipped", + report.summary.ok, report.summary.warnings, report.summary.errors, report.summary.skipped + ); + println!(); + + if report.has_errors() { + println!(" Result: FAILED — errors found"); + } else if report.has_warnings() { + println!( + " Result: OK — {} warning(s) found", + report.summary.warnings + ); + } else if report.summary.ok == 0 && report.summary.skipped > 0 { + println!(" Result: SKIPPED — no checks could run"); + } else { + println!(" Result: OK — all checks passed"); + } + println!(); +} + +fn print_finding(f: &DoctorFinding) { + println!(" {} {}: {}", status_icon(f.status), f.title, f.detail); + if let Some(ref suggestion) = f.suggestion { + println!(" → {}", suggestion); + } +} + +fn status_icon(status: DoctorStatus) -> &'static str { + match status { + DoctorStatus::Ok => "✓", + DoctorStatus::Warning => "⚠", + DoctorStatus::Error => "✗", + DoctorStatus::Skipped => "○", + } +} diff --git a/ows/crates/ows-cli/src/commands/doctor/report.rs b/ows/crates/ows-cli/src/commands/doctor/report.rs index 476ad01a..43688569 100644 --- a/ows/crates/ows-cli/src/commands/doctor/report.rs +++ b/ows/crates/ows-cli/src/commands/doctor/report.rs @@ -168,10 +168,22 @@ impl DoctorReport { /// - `Skipped` if only skipped findings pub fn new(findings: Vec) -> Self { let summary = DoctorSummary { - ok: findings.iter().filter(|f| f.status == DoctorStatus::Ok).count(), - warnings: findings.iter().filter(|f| f.status == DoctorStatus::Warning).count(), - errors: findings.iter().filter(|f| f.status == DoctorStatus::Error).count(), - skipped: findings.iter().filter(|f| f.status == DoctorStatus::Skipped).count(), + ok: findings + .iter() + .filter(|f| f.status == DoctorStatus::Ok) + .count(), + warnings: findings + .iter() + .filter(|f| f.status == DoctorStatus::Warning) + .count(), + errors: findings + .iter() + .filter(|f| f.status == DoctorStatus::Error) + .count(), + skipped: findings + .iter() + .filter(|f| f.status == DoctorStatus::Skipped) + .count(), }; let overall_status = if summary.errors > 0 { @@ -212,7 +224,10 @@ impl DoctorReport { /// Return findings filtered by a given status. pub fn findings_with_status(&self, status: DoctorStatus) -> Vec<&DoctorFinding> { - self.findings.iter().filter(|f| f.status == status).collect() + self.findings + .iter() + .filter(|f| f.status == status) + .collect() } } @@ -310,13 +325,16 @@ mod tests { let finding = DoctorFinding::ok(ID, "Title", "Detail") .with_path(std::path::PathBuf::from("/test/path")); assert!(finding.path.is_some()); - assert_eq!(finding.path.unwrap(), std::path::PathBuf::from("/test/path")); + assert_eq!( + finding.path.unwrap(), + std::path::PathBuf::from("/test/path") + ); } #[test] fn test_finding_builder_with_code() { - let finding = DoctorFinding::error(ID, "Title", "Detail", "Fix it") - .with_code("ERR_VAULT_MISSING"); + let finding = + DoctorFinding::error(ID, "Title", "Detail", "Fix it").with_code("ERR_VAULT_MISSING"); assert_eq!(finding.code, Some("ERR_VAULT_MISSING")); } diff --git a/ows/crates/ows-cli/src/commands/doctor/vault_inspector.rs b/ows/crates/ows-cli/src/commands/doctor/vault_inspector.rs index c81b0c8d..46a57c1e 100644 --- a/ows/crates/ows-cli/src/commands/doctor/vault_inspector.rs +++ b/ows/crates/ows-cli/src/commands/doctor/vault_inspector.rs @@ -103,14 +103,16 @@ pub fn check_wallet_files(vault_path: &Path) -> Vec { Ok(c) => c, Err(e) => { corrupted_count += 1; - findings.push(DoctorFinding::error( - CHECK_WALLET_FILES, - "Wallet file unreadable", - &format!("{}: cannot read file: {}", file_name, e), - "Check file permissions.", - ) - .with_path(path) - .with_code("ERR_FILE_UNREADABLE")); + findings.push( + DoctorFinding::error( + CHECK_WALLET_FILES, + "Wallet file unreadable", + &format!("{}: cannot read file: {}", file_name, e), + "Check file permissions.", + ) + .with_path(path) + .with_code("ERR_FILE_UNREADABLE"), + ); continue; } }; @@ -134,41 +136,50 @@ pub fn check_wallet_files(vault_path: &Path) -> Vec { // Additional metadata validation if wallet.id.is_empty() { - findings.push(DoctorFinding::error( - CHECK_WALLET_FILES, - "Wallet has empty ID", - &format!("{}: wallet ID field is empty", file_name), - "Recreate the wallet from the mnemonic.", - ) - .with_path(path) - .with_code("ERR_METADATA_INVALID")); + findings.push( + DoctorFinding::error( + CHECK_WALLET_FILES, + "Wallet has empty ID", + &format!("{}: wallet ID field is empty", file_name), + "Recreate the wallet from the mnemonic.", + ) + .with_path(path) + .with_code("ERR_METADATA_INVALID"), + ); corrupted_count += 1; continue; } if wallet.created_at.is_empty() { - findings.push(DoctorFinding::error( - CHECK_WALLET_FILES, - "Wallet has empty created_at", - &format!("{}: created_at field is empty", file_name), - "Recreate the wallet from the mnemonic.", - ) - .with_path(path) - .with_code("ERR_METADATA_INVALID")); + findings.push( + DoctorFinding::error( + CHECK_WALLET_FILES, + "Wallet has empty created_at", + &format!("{}: created_at field is empty", file_name), + "Recreate the wallet from the mnemonic.", + ) + .with_path(path) + .with_code("ERR_METADATA_INVALID"), + ); corrupted_count += 1; continue; } // Validate created_at is valid RFC3339 if chrono::DateTime::parse_from_rfc3339(&wallet.created_at).is_err() { - findings.push(DoctorFinding::error( - CHECK_WALLET_FILES, - "Wallet has invalid created_at", - &format!("{}: created_at is not valid RFC3339: `{}`", file_name, wallet.created_at), - "Recreate the wallet from the mnemonic.", - ) - .with_path(path) - .with_code("ERR_METADATA_INVALID")); + findings.push( + DoctorFinding::error( + CHECK_WALLET_FILES, + "Wallet has invalid created_at", + &format!( + "{}: created_at is not valid RFC3339: `{}`", + file_name, wallet.created_at + ), + "Recreate the wallet from the mnemonic.", + ) + .with_path(path) + .with_code("ERR_METADATA_INVALID"), + ); corrupted_count += 1; continue; } @@ -184,17 +195,19 @@ pub fn check_wallet_files(vault_path: &Path) -> Vec { &format!("All {} wallet file(s) are valid.", valid_count), )); } else { - findings.push(DoctorFinding::warning( - CHECK_WALLET_FILES, - "Some wallet files corrupted", - &format!( - "{} of {} wallet file(s) are corrupted.", - corrupted_count, - valid_count + corrupted_count - ), - "Export valid wallets and recreate the corrupted ones.", - ) - .with_code("WARN_ARTIFACTS_CORRUPTED")); + findings.push( + DoctorFinding::warning( + CHECK_WALLET_FILES, + "Some wallet files corrupted", + &format!( + "{} of {} wallet file(s) are corrupted.", + corrupted_count, + valid_count + corrupted_count + ), + "Export valid wallets and recreate the corrupted ones.", + ) + .with_code("WARN_ARTIFACTS_CORRUPTED"), + ); } findings @@ -258,14 +271,16 @@ pub fn check_key_files(vault_path: &Path) -> Vec { Ok(c) => c, Err(e) => { corrupted_count += 1; - findings.push(DoctorFinding::error( - CHECK_KEY_FILES, - "Key file unreadable", - &format!("{}: cannot read file: {}", file_name, e), - "Check file permissions.", - ) - .with_path(path) - .with_code("ERR_FILE_UNREADABLE")); + findings.push( + DoctorFinding::error( + CHECK_KEY_FILES, + "Key file unreadable", + &format!("{}: cannot read file: {}", file_name, e), + "Check file permissions.", + ) + .with_path(path) + .with_code("ERR_FILE_UNREADABLE"), + ); continue; } }; @@ -274,14 +289,16 @@ pub fn check_key_files(vault_path: &Path) -> Vec { Ok(k) => k, Err(e) => { corrupted_count += 1; - findings.push(DoctorFinding::error( - CHECK_KEY_FILES, - "Key file malformed", - &format!("{}: invalid JSON: {}", file_name, e), - "Delete and recreate the API key.", - ) - .with_path(path) - .with_code("ERR_FILE_MALFORMED")); + findings.push( + DoctorFinding::error( + CHECK_KEY_FILES, + "Key file malformed", + &format!("{}: invalid JSON: {}", file_name, e), + "Delete and recreate the API key.", + ) + .with_path(path) + .with_code("ERR_FILE_MALFORMED"), + ); continue; } }; @@ -296,17 +313,19 @@ pub fn check_key_files(vault_path: &Path) -> Vec { &format!("All {} key file(s) are valid.", valid_count), )); } else { - findings.push(DoctorFinding::warning( - CHECK_KEY_FILES, - "Some key files corrupted", - &format!( - "{} of {} key file(s) are corrupted.", - corrupted_count, - valid_count + corrupted_count - ), - "Delete and recreate the corrupted API keys.", - ) - .with_code("WARN_ARTIFACTS_CORRUPTED")); + findings.push( + DoctorFinding::warning( + CHECK_KEY_FILES, + "Some key files corrupted", + &format!( + "{} of {} key file(s) are corrupted.", + corrupted_count, + valid_count + corrupted_count + ), + "Delete and recreate the corrupted API keys.", + ) + .with_code("WARN_ARTIFACTS_CORRUPTED"), + ); } findings @@ -370,14 +389,16 @@ pub fn check_policy_files(vault_path: &Path) -> Vec { Ok(c) => c, Err(e) => { corrupted_count += 1; - findings.push(DoctorFinding::error( - CHECK_POLICY_FILES, - "Policy file unreadable", - &format!("{}: cannot read file: {}", file_name, e), - "Check file permissions.", - ) - .with_path(path) - .with_code("ERR_FILE_UNREADABLE")); + findings.push( + DoctorFinding::error( + CHECK_POLICY_FILES, + "Policy file unreadable", + &format!("{}: cannot read file: {}", file_name, e), + "Check file permissions.", + ) + .with_path(path) + .with_code("ERR_FILE_UNREADABLE"), + ); continue; } }; @@ -386,14 +407,16 @@ pub fn check_policy_files(vault_path: &Path) -> Vec { Ok(p) => p, Err(e) => { corrupted_count += 1; - findings.push(DoctorFinding::error( - CHECK_POLICY_FILES, - "Policy file malformed", - &format!("{}: invalid JSON: {}", file_name, e), - "Delete and recreate the policy.", - ) - .with_path(path) - .with_code("ERR_FILE_MALFORMED")); + findings.push( + DoctorFinding::error( + CHECK_POLICY_FILES, + "Policy file malformed", + &format!("{}: invalid JSON: {}", file_name, e), + "Delete and recreate the policy.", + ) + .with_path(path) + .with_code("ERR_FILE_MALFORMED"), + ); continue; } }; @@ -408,17 +431,19 @@ pub fn check_policy_files(vault_path: &Path) -> Vec { &format!("All {} policy file(s) are valid.", valid_count), )); } else { - findings.push(DoctorFinding::warning( - CHECK_POLICY_FILES, - "Some policy files corrupted", - &format!( - "{} of {} policy file(s) are corrupted.", - corrupted_count, - valid_count + corrupted_count - ), - "Delete and recreate the corrupted policies.", - ) - .with_code("WARN_ARTIFACTS_CORRUPTED")); + findings.push( + DoctorFinding::warning( + CHECK_POLICY_FILES, + "Some policy files corrupted", + &format!( + "{} of {} policy file(s) are corrupted.", + corrupted_count, + valid_count + corrupted_count + ), + "Delete and recreate the corrupted policies.", + ) + .with_code("WARN_ARTIFACTS_CORRUPTED"), + ); } findings @@ -480,7 +505,7 @@ mod tests { fn test_wallet_files_empty_dir_is_warning() { let temp = TempDir::new().unwrap(); let vault = temp.path().join(".ows"); - std::fs::create_dir(vault.join("wallets")).ok(); + std::fs::create_dir_all(vault.join("wallets")).ok(); let findings = check_wallet_files(&vault); assert_eq!(findings.len(), 1); assert_eq!(findings[0].status, DoctorStatus::Warning); @@ -499,8 +524,12 @@ mod tests { let findings = check_wallet_files(&vault); // Should have 2 findings: one for the wallet, one summary - assert!(findings.iter().any(|f| f.status == DoctorStatus::Ok && f.detail.contains("1 wallet"))); - assert!(findings.iter().any(|f| f.id == CHECK_WALLET_FILES && f.status == DoctorStatus::Ok)); + assert!(findings + .iter() + .any(|f| f.status == DoctorStatus::Ok && f.detail.contains("1 wallet"))); + assert!(findings + .iter() + .any(|f| f.id == CHECK_WALLET_FILES && f.status == DoctorStatus::Ok)); } #[test] @@ -512,7 +541,9 @@ mod tests { std::fs::write(wallets_dir.join("bad.json"), "{ invalid json }").ok(); let findings = check_wallet_files(&vault); - assert!(findings.iter().any(|f| f.status == DoctorStatus::Error && f.code == Some("ERR_FILE_MALFORMED"))); + assert!(findings + .iter() + .any(|f| f.status == DoctorStatus::Error && f.code == Some("ERR_FILE_MALFORMED"))); } #[test] @@ -526,29 +557,39 @@ mod tests { std::fs::write(wallets_dir.join("empty-id.json"), json).ok(); let findings = check_wallet_files(&vault); - assert!(findings.iter().any(|f| f.code == Some("ERR_METADATA_INVALID"))); + assert!(findings + .iter() + .any(|f| f.code == Some("ERR_METADATA_INVALID"))); } #[test] fn test_wallet_files_invalid_created_at() { + // Create a wallet JSON with an invalid created_at RFC3339 string let temp = TempDir::new().unwrap(); let vault = temp.path().join(".ows"); let wallets_dir = vault.join("wallets"); std::fs::create_dir_all(&wallets_dir).ok(); - let wallet = EncryptedWallet::new( - "test-id".to_string(), - "Test".to_string(), - vec![], - serde_json::json!({}), - ows_core::KeyType::Mnemonic, - ); - // Override created_at to be invalid - let mut json = serde_json::to_string_pretty(&wallet).unwrap(); - json = json.replace("2026-01-01T00:00:00Z", "not-a-date"); - std::fs::write(wallets_dir.join("bad-date.json"), json).ok(); + + let bad_wallet_json = serde_json::json!({ + "ows_version": 2, + "id": "test-id", + "name": "Test", + "created_at": "not-a-date", + "accounts": [], + "crypto": {}, + "key_type": "mnemonic" + }); + std::fs::write( + wallets_dir.join("bad-date.json"), + serde_json::to_string_pretty(&bad_wallet_json).unwrap(), + ) + .ok(); let findings = check_wallet_files(&vault); - assert!(findings.iter().any(|f| f.code == Some("ERR_METADATA_INVALID"))); + // Should detect invalid created_at + assert!(findings + .iter() + .any(|f| f.code == Some("ERR_METADATA_INVALID"))); } #[test] @@ -563,13 +604,15 @@ mod tests { let json = serde_json::to_string_pretty(&wallet).unwrap(); std::fs::write(wallets_dir.join("good.json"), json).ok(); - // Corrupted wallet + // Corrupted wallet (malformed JSON) std::fs::write(wallets_dir.join("bad.json"), "{ bad }").ok(); let findings = check_wallet_files(&vault); - assert!(findings.iter().any(|f| f.status == DoctorStatus::Ok)); + // With mixed valid and corrupted: one Error (malformed), one Warning (summary) assert!(findings.iter().any(|f| f.status == DoctorStatus::Error)); - assert!(findings.iter().any(|f| f.code == Some("WARN_ARTIFACTS_CORRUPTED"))); + assert!(findings + .iter() + .any(|f| f.code == Some("WARN_ARTIFACTS_CORRUPTED"))); } // ---- Key file tests ---- @@ -615,7 +658,9 @@ mod tests { std::fs::write(keys_dir.join("bad.json"), "{ invalid }").ok(); let findings = check_key_files(&vault); - assert!(findings.iter().any(|f| f.status == DoctorStatus::Error && f.code == Some("ERR_FILE_MALFORMED"))); + assert!(findings + .iter() + .any(|f| f.status == DoctorStatus::Error && f.code == Some("ERR_FILE_MALFORMED"))); } // ---- Policy file tests ---- @@ -661,6 +706,8 @@ mod tests { std::fs::write(policies_dir.join("bad.json"), "{ invalid }").ok(); let findings = check_policy_files(&vault); - assert!(findings.iter().any(|f| f.status == DoctorStatus::Error && f.code == Some("ERR_FILE_MALFORMED"))); + assert!(findings + .iter() + .any(|f| f.status == DoctorStatus::Error && f.code == Some("ERR_FILE_MALFORMED"))); } } diff --git a/ows/crates/ows-cli/src/main.rs b/ows/crates/ows-cli/src/main.rs index d106319e..b6454876 100644 --- a/ows/crates/ows-cli/src/main.rs +++ b/ows/crates/ows-cli/src/main.rs @@ -70,6 +70,8 @@ enum Commands { #[arg(long)] purge: bool, }, + /// Run diagnostic checks on the OWS installation + Doctor, } #[derive(Subcommand)] @@ -512,5 +514,6 @@ fn run(cli: Cli) -> Result<(), CliError> { }, Commands::Update { force } => commands::update::run(force), Commands::Uninstall { purge } => commands::uninstall::run(purge), + Commands::Doctor => commands::doctor::run(), } } From 5d01cc4aff5f66cf88e5d34ac38bb49854644ba2 Mon Sep 17 00:00:00 2001 From: OKWN Date: Thu, 2 Apr 2026 13:36:43 +0300 Subject: [PATCH 5/9] feat(ows-cli): formalize finding taxonomy with stable OWS_DOCTOR_* codes Phase 5 makes diagnostics consistent, actionable, and maintainable: - Add 23 stable `OWS_DOCTOR_*` constants as the canonical finding codes: OWS_DOCTOR_ENV_HOME_NOT_SET, OWS_DOCTOR_VAULT_MISSING, OWS_DOCTOR_LOGS_DIR_MISSING, OWS_DOCTOR_CONFIG_MISSING, OWS_DOCTOR_CONFIG_INVALID, OWS_DOCTOR_DIR_UNREADABLE, OWS_DOCTOR_PERM_VAULT_INSECURE, OWS_DOCTOR_PERM_WALLETS_INSECURE, OWS_DOCTOR_PERM_WALLET_FILE_INSECURE, OWS_DOCTOR_WALLET_NONE, OWS_DOCTOR_WALLET_FILE_UNREADABLE, OWS_DOCTOR_WALLET_FILE_INVALID, OWS_DOCTOR_WALLET_METADATA_CORRUPT, OWS_DOCTOR_WALLET_SOME_CORRUPT, OWS_DOCTOR_POLICY_NONE, OWS_DOCTOR_POLICY_FILE_UNREADABLE, OWS_DOCTOR_POLICY_FILE_INVALID, OWS_DOCTOR_POLICY_SOME_CORRUPT, OWS_DOCTOR_KEY_NONE, OWS_DOCTOR_KEY_FILE_UNREADABLE, OWS_DOCTOR_KEY_FILE_INVALID, OWS_DOCTOR_KEY_SOME_CORRUPT - Change DoctorFinding builders: skipped/warning/error now require a stable code as the second argument. ok() remains code-free. - Every Error, Warning, and Skipped finding now carries a machine-stable code suitable for future JSON output and programmatic handling. - Wording polished: operator-facing titles and detail text revised for clarity, precision, and actionability. Suggestions updated to be concrete and CLI-ready. - Tests updated: all 31 doctor tests assert stable codes where appropriate. Co-Authored-By: Claude Opus 4.6 --- .../ows-cli/src/commands/doctor/checks.rs | 80 ++-- .../ows-cli/src/commands/doctor/report.rs | 171 ++++++--- .../src/commands/doctor/vault_inspector.rs | 363 ++++++++---------- 3 files changed, 342 insertions(+), 272 deletions(-) diff --git a/ows/crates/ows-cli/src/commands/doctor/checks.rs b/ows/crates/ows-cli/src/commands/doctor/checks.rs index 2f8f0552..a78bc231 100644 --- a/ows/crates/ows-cli/src/commands/doctor/checks.rs +++ b/ows/crates/ows-cli/src/commands/doctor/checks.rs @@ -1,6 +1,10 @@ //! Individual diagnostic checks for `ows doctor`. -use crate::commands::doctor::report::{DoctorCheckId, DoctorFinding, DoctorReport}; +use crate::commands::doctor::report::{ + DoctorCheckId, DoctorFinding, DoctorReport, OWS_DOCTOR_CONFIG_INVALID, + OWS_DOCTOR_CONFIG_MISSING, OWS_DOCTOR_DIR_UNREADABLE, OWS_DOCTOR_ENV_HOME_NOT_SET, + OWS_DOCTOR_LOGS_DIR_MISSING, OWS_DOCTOR_VAULT_MISSING, +}; use crate::commands::doctor::vault_inspector; use ows_core::Config; @@ -45,9 +49,10 @@ pub fn check_vault_path() -> Vec { if home.is_none() { findings.push(DoctorFinding::error( CHECK_HOME_ENV, - "HOME not set", - "The HOME environment variable is not set. Vault path resolution may be incorrect.", - "Set HOME to your user directory path.", + OWS_DOCTOR_ENV_HOME_NOT_SET, + "HOME environment variable is not set", + "Vault path resolution may be unreliable without HOME. Set HOME to your user directory.", + "Set the HOME environment variable (e.g. HOME=~/.ows in your shell profile).", )); } @@ -56,7 +61,7 @@ pub fn check_vault_path() -> Vec { findings.push(DoctorFinding::ok( CHECK_VAULT_PATH, "Vault path resolved", - &format!("Vault path resolved to `{}`", vault_path.display()), + &format!("Vault path resolved to `{}`.", vault_path.display()), )); findings @@ -70,14 +75,15 @@ pub fn check_vault_exists() -> Vec { vec![DoctorFinding::ok( CHECK_VAULT_EXISTS, "Vault exists", - &format!("`{}` exists", vault_path.display()), + &format!("Vault directory is present at `{}`.", vault_path.display()), )] } else { vec![DoctorFinding::error( CHECK_VAULT_EXISTS, - "Vault not found", + OWS_DOCTOR_VAULT_MISSING, + "Vault directory not found", &format!( - "Vault directory not found at `{}`. No wallets have been created yet.", + "No vault found at `{}`. No wallets have been created yet.", vault_path.display() ), "Run `ows wallet create` to create your first wallet.", @@ -93,7 +99,8 @@ pub fn check_logs_dir() -> Vec { if !vault_path.exists() { return vec![DoctorFinding::skipped( CHECK_LOGS_DIR, - "Logs directory skipped", + OWS_DOCTOR_LOGS_DIR_MISSING, + "Logs directory check skipped", "Vault does not exist; skipping logs directory check.", )]; } @@ -104,12 +111,13 @@ pub fn check_logs_dir() -> Vec { vec![DoctorFinding::ok( CHECK_LOGS_DIR, "Logs directory present", - &format!("logs/ exists at `{}`", logs_dir.display()), + &format!("Audit log directory exists at `{}`.", logs_dir.display()), ) .with_path(logs_dir)] } else { vec![DoctorFinding::skipped( CHECK_LOGS_DIR, + OWS_DOCTOR_LOGS_DIR_MISSING, "Logs directory not present", "logs/ does not exist. Audit logging may not be active.", )] @@ -123,8 +131,9 @@ pub fn check_config() -> Vec { if !config_path.exists() { return vec![DoctorFinding::skipped( CHECK_CONFIG, + OWS_DOCTOR_CONFIG_MISSING, "Config file not present", - "No user config file at `~/.ows/config.json`. Using built-in defaults.", + "No user config at `~/.ows/config.json`. Built-in defaults are in use.", )]; } @@ -134,9 +143,9 @@ pub fn check_config() -> Vec { let rpc_count = config.rpc.len(); vec![DoctorFinding::ok( CHECK_CONFIG, - "Config valid", + "Config file valid", &format!( - "config.json is valid with {} RPC endpoint(s) configured", + "config.json is valid with {} RPC endpoint(s) configured.", rpc_count ), ) @@ -145,12 +154,12 @@ pub fn check_config() -> Vec { Err(e) => { vec![DoctorFinding::error( CHECK_CONFIG, - "Config parse error", - &format!("config.json exists but failed to parse: {}", e), + OWS_DOCTOR_CONFIG_INVALID, + "Config file is invalid", + &format!("config.json is present but could not be parsed: {}.", e), "Backup and recreate `~/.ows/config.json`.", ) - .with_path(config_path) - .with_code("ERR_CONFIG_PARSE")] + .with_path(config_path)] } } } @@ -158,6 +167,10 @@ pub fn check_config() -> Vec { /// Check vault directory permissions (Unix only). #[cfg(unix)] pub fn check_vault_permissions() -> Vec { + use crate::commands::doctor::report::{ + OWS_DOCTOR_PERM_VAULT_INSECURE, OWS_DOCTOR_PERM_WALLET_FILE_INSECURE, + OWS_DOCTOR_PERM_WALLETS_INSECURE, + }; use std::os::unix::fs::PermissionsExt; let vault_path = resolve_vault_path(); @@ -165,6 +178,7 @@ pub fn check_vault_permissions() -> Vec { if !vault_path.exists() { return vec![DoctorFinding::skipped( CHECK_VAULT_PERMS, + OWS_DOCTOR_DIR_UNREADABLE, "Permissions check skipped", "Vault does not exist; skipping permissions check.", )]; @@ -179,22 +193,22 @@ pub fn check_vault_permissions() -> Vec { findings.push( DoctorFinding::warning( CHECK_VAULT_PERMS, - "Vault permissions too open", + OWS_DOCTOR_PERM_VAULT_INSECURE, + "Vault directory permissions are insecure", &format!( - "Vault directory has permissions {:03o}, expected 0700 (owner-only)", + "Vault has mode {:03o}; owner-only (0700) is required.", mode ), "Run: chmod 700 ~/.ows", ) - .with_path(vault_path.clone()) - .with_code("WARN_VAULT_PERMS"), + .with_path(vault_path.clone()), ); } else { findings.push( DoctorFinding::ok( CHECK_VAULT_PERMS, - "Vault permissions correct", - "Vault directory has correct permissions (0700).", + "Vault directory permissions are correct", + "Vault directory has secure permissions (0700).", ) .with_path(vault_path.clone()), ); @@ -209,15 +223,15 @@ pub fn check_vault_permissions() -> Vec { findings.push( DoctorFinding::warning( CHECK_VAULT_PERMS, - "Wallets directory permissions too open", + OWS_DOCTOR_PERM_WALLETS_INSECURE, + "Wallets directory permissions are insecure", &format!( - "Wallets directory has permissions {:03o}, expected 0700", + "wallets/ has mode {:03o}; owner-only (0700) is required.", mode ), "Run: chmod 700 ~/.ows/wallets", ) - .with_path(wallets_dir.clone()) - .with_code("WARN_WALLETS_PERMS"), + .with_path(wallets_dir.clone()), ); } } @@ -233,15 +247,15 @@ pub fn check_vault_permissions() -> Vec { findings.push( DoctorFinding::warning( CHECK_VAULT_PERMS, - "Wallet file permissions too open", + OWS_DOCTOR_PERM_WALLET_FILE_INSECURE, + "Wallet file permissions are insecure", &format!( - "{} has permissions {:03o}, expected 0600", + "{} has mode {:03o}; owner-read-only (0600) is required.", file_name, mode ), &format!("Run: chmod 600 ~/.ows/wallets/{}", file_name), ) - .with_path(entry.path()) - .with_code("WARN_WALLET_FILE_PERMS"), + .with_path(entry.path()), ); } } @@ -265,6 +279,7 @@ pub fn check_vault_permissions() -> Vec { pub fn check_vault_permissions() -> Vec { vec![DoctorFinding::skipped( CHECK_VAULT_PERMS, + OWS_DOCTOR_DIR_UNREADABLE, "Permissions check skipped", "Permission checks are Unix-only.", )] @@ -382,6 +397,7 @@ mod tests { #[test] fn test_wallet_inspection_malformed_json() { + use crate::commands::doctor::report::OWS_DOCTOR_WALLET_FILE_INVALID; let temp = tempfile::TempDir::new().unwrap(); let vault = temp.path().to_path_buf(); let wallets_dir = vault.join("wallets"); @@ -391,7 +407,7 @@ mod tests { let findings = vault_inspector::check_wallet_files(&vault); assert!(findings .iter() - .any(|f| f.code == Some("ERR_FILE_MALFORMED"))); + .any(|f| f.code == Some(OWS_DOCTOR_WALLET_FILE_INVALID))); } #[test] diff --git a/ows/crates/ows-cli/src/commands/doctor/report.rs b/ows/crates/ows-cli/src/commands/doctor/report.rs index 43688569..d2d7f388 100644 --- a/ows/crates/ows-cli/src/commands/doctor/report.rs +++ b/ows/crates/ows-cli/src/commands/doctor/report.rs @@ -2,12 +2,78 @@ use std::fmt; +// --------------------------------------------------------------------------- +// Stable finding codes +// --------------------------------------------------------------------------- + +// Taxonomy: every actionable finding carries a stable code. Ok findings +// (purely informational, no action needed) do not use codes. +// +// Prefix map: +// OWS_DOCTOR_ENV_* — environment / path resolution +// OWS_DOCTOR_VAULT_* — vault-level structural checks +// OWS_DOCTOR_CONFIG_* — config file parsing +// OWS_DOCTOR_PERM_* — Unix file permissions +// OWS_DOCTOR_WALLET_* — wallet file validation +// OWS_DOCTOR_POLICY_* — policy file validation +// OWS_DOCTOR_KEY_* — API key file validation + +/// HOME environment variable is not set. +pub const OWS_DOCTOR_ENV_HOME_NOT_SET: &str = "OWS_DOCTOR_ENV_HOME_NOT_SET"; +/// Vault directory does not exist. +pub const OWS_DOCTOR_VAULT_MISSING: &str = "OWS_DOCTOR_VAULT_MISSING"; +/// Vault logs subdirectory is absent. +pub const OWS_DOCTOR_LOGS_DIR_MISSING: &str = "OWS_DOCTOR_LOGS_DIR_MISSING"; +/// Config file is absent; built-in defaults are in use. +pub const OWS_DOCTOR_CONFIG_MISSING: &str = "OWS_DOCTOR_CONFIG_MISSING"; +/// Config file is present but malformed (invalid JSON or schema). +pub const OWS_DOCTOR_CONFIG_INVALID: &str = "OWS_DOCTOR_CONFIG_INVALID"; +/// A vault subdirectory cannot be read due to permissions or I/O errors. +pub const OWS_DOCTOR_DIR_UNREADABLE: &str = "OWS_DOCTOR_DIR_UNREADABLE"; +/// Vault directory permissions are insecure (Unix). +#[allow(dead_code)] +pub const OWS_DOCTOR_PERM_VAULT_INSECURE: &str = "OWS_DOCTOR_PERM_VAULT_INSECURE"; +/// wallets/ directory permissions are insecure (Unix). +#[allow(dead_code)] +pub const OWS_DOCTOR_PERM_WALLETS_INSECURE: &str = "OWS_DOCTOR_PERM_WALLETS_INSECURE"; +/// A wallet file has permissions insecure for a secret file (Unix). +#[allow(dead_code)] +pub const OWS_DOCTOR_PERM_WALLET_FILE_INSECURE: &str = "OWS_DOCTOR_PERM_WALLET_FILE_INSECURE"; +/// No wallet files present in the vault. +pub const OWS_DOCTOR_WALLET_NONE: &str = "OWS_DOCTOR_WALLET_NONE"; +/// A wallet file cannot be read. +pub const OWS_DOCTOR_WALLET_FILE_UNREADABLE: &str = "OWS_DOCTOR_WALLET_FILE_UNREADABLE"; +/// A wallet file is not valid JSON. +pub const OWS_DOCTOR_WALLET_FILE_INVALID: &str = "OWS_DOCTOR_WALLET_FILE_INVALID"; +/// A wallet file has invalid or missing metadata (empty ID, empty/invalid created_at). +pub const OWS_DOCTOR_WALLET_METADATA_CORRUPT: &str = "OWS_DOCTOR_WALLET_METADATA_CORRUPT"; +/// Some wallet files are corrupted while others are valid. +pub const OWS_DOCTOR_WALLET_SOME_CORRUPT: &str = "OWS_DOCTOR_WALLET_SOME_CORRUPT"; +/// No policy files present. +pub const OWS_DOCTOR_POLICY_NONE: &str = "OWS_DOCTOR_POLICY_NONE"; +/// A policy file cannot be read. +pub const OWS_DOCTOR_POLICY_FILE_UNREADABLE: &str = "OWS_DOCTOR_POLICY_FILE_UNREADABLE"; +/// A policy file is not valid JSON. +pub const OWS_DOCTOR_POLICY_FILE_INVALID: &str = "OWS_DOCTOR_POLICY_FILE_INVALID"; +/// Some policy files are corrupted while others are valid. +pub const OWS_DOCTOR_POLICY_SOME_CORRUPT: &str = "OWS_DOCTOR_POLICY_SOME_CORRUPT"; +/// No API key files present. +pub const OWS_DOCTOR_KEY_NONE: &str = "OWS_DOCTOR_KEY_NONE"; +/// An API key file cannot be read. +pub const OWS_DOCTOR_KEY_FILE_UNREADABLE: &str = "OWS_DOCTOR_KEY_FILE_UNREADABLE"; +/// An API key file is not valid JSON. +pub const OWS_DOCTOR_KEY_FILE_INVALID: &str = "OWS_DOCTOR_KEY_FILE_INVALID"; +/// Some API key files are corrupted while others are valid. +pub const OWS_DOCTOR_KEY_SOME_CORRUPT: &str = "OWS_DOCTOR_KEY_SOME_CORRUPT"; + +// --------------------------------------------------------------------------- +// Check IDs +// --------------------------------------------------------------------------- + /// Unique identifier for a diagnostic check. /// -/// Codes are stable, human-readable identifiers used for: -/// - Structured output (future JSON mode) -/// - Test identification -/// - Cross-referencing findings +/// Check IDs are stable, dotted identifiers used to group findings +/// and as a stable anchor for structured output (future JSON mode). #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub struct DoctorCheckId(&'static str); @@ -27,6 +93,10 @@ impl fmt::Display for DoctorCheckId { } } +// --------------------------------------------------------------------------- +// Status +// -------------------------------------------------------------------------- + /// Status of a single diagnostic check. #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[non_exhaustive] @@ -37,15 +107,14 @@ pub enum DoctorStatus { Warning, /// Check failed; a problem requires attention. Error, - /// Check was skipped because it does not apply (e.g., no wallets exist). + /// Check was skipped because it does not apply on this platform + /// or because the prerequisite state is absent. Skipped, } -impl DoctorStatus { - pub fn is_passing(&self) -> bool { - matches!(self, DoctorStatus::Ok | DoctorStatus::Skipped) - } -} +// --------------------------------------------------------------------------- +// Finding +// -------------------------------------------------------------------------- /// A single diagnostic finding from one check. #[derive(Debug, Clone)] @@ -62,11 +131,13 @@ pub struct DoctorFinding { pub suggestion: Option, /// Path to the file or directory involved, if applicable. pub path: Option, - /// Stable error code for machine processing (future JSON output). + /// Stable code for machine processing. Always present for Error, + /// Warning, and Skipped findings. Absent for informational Ok findings. pub code: Option<&'static str>, } impl DoctorFinding { + /// Create an informational Ok finding (no code needed). pub fn ok(id: DoctorCheckId, title: &str, detail: &str) -> Self { DoctorFinding { id, @@ -79,7 +150,8 @@ impl DoctorFinding { } } - pub fn skipped(id: DoctorCheckId, title: &str, detail: &str) -> Self { + /// Create a Skipped finding with a stable code. + pub fn skipped(id: DoctorCheckId, code: &'static str, title: &str, detail: &str) -> Self { DoctorFinding { id, status: DoctorStatus::Skipped, @@ -87,11 +159,18 @@ impl DoctorFinding { detail: detail.to_string(), suggestion: None, path: None, - code: None, + code: Some(code), } } - pub fn warning(id: DoctorCheckId, title: &str, detail: &str, suggestion: &str) -> Self { + /// Create a Warning finding with a stable code. + pub fn warning( + id: DoctorCheckId, + code: &'static str, + title: &str, + detail: &str, + suggestion: &str, + ) -> Self { DoctorFinding { id, status: DoctorStatus::Warning, @@ -99,11 +178,18 @@ impl DoctorFinding { detail: detail.to_string(), suggestion: Some(suggestion.to_string()), path: None, - code: None, + code: Some(code), } } - pub fn error(id: DoctorCheckId, title: &str, detail: &str, suggestion: &str) -> Self { + /// Create an Error finding with a stable code. + pub fn error( + id: DoctorCheckId, + code: &'static str, + title: &str, + detail: &str, + suggestion: &str, + ) -> Self { DoctorFinding { id, status: DoctorStatus::Error, @@ -111,7 +197,7 @@ impl DoctorFinding { detail: detail.to_string(), suggestion: Some(suggestion.to_string()), path: None, - code: None, + code: Some(code), } } @@ -120,12 +206,6 @@ impl DoctorFinding { self.path = Some(path); self } - - /// Builder-style method to attach an error code to the finding. - pub fn with_code(mut self, code: &'static str) -> Self { - self.code = Some(code); - self - } } /// Summary counts across all findings. @@ -241,8 +321,8 @@ mod tests { fn test_summary_counts() { let findings = vec![ DoctorFinding::ok(ID, "Good", "All good"), - DoctorFinding::skipped(ID, "Skipped", "Not applicable"), - DoctorFinding::warning(ID, "Warn", "Minor issue", "Fix it"), + DoctorFinding::skipped(ID, OWS_DOCTOR_VAULT_MISSING, "Skipped", "Not applicable"), + DoctorFinding::warning(ID, OWS_DOCTOR_WALLET_NONE, "Warn", "Minor issue", "Fix it"), ]; let report = DoctorReport::new(findings); assert_eq!(report.summary.ok, 1); @@ -256,8 +336,8 @@ mod tests { fn test_overall_status_error_wins() { let findings = vec![ DoctorFinding::ok(ID, "Good", "All good"), - DoctorFinding::error(ID, "Bad", "Critical", "Fix it"), - DoctorFinding::warning(ID, "Warn", "Minor", "Fix it"), + DoctorFinding::error(ID, OWS_DOCTOR_VAULT_MISSING, "Bad", "Critical", "Fix it"), + DoctorFinding::warning(ID, OWS_DOCTOR_WALLET_NONE, "Warn", "Minor", "Fix it"), ]; let report = DoctorReport::new(findings); assert_eq!(report.overall_status, DoctorStatus::Error); @@ -270,7 +350,7 @@ mod tests { fn test_overall_status_warning_without_error() { let findings = vec![ DoctorFinding::ok(ID, "Good", "All good"), - DoctorFinding::warning(ID, "Warn", "Minor", "Fix it"), + DoctorFinding::warning(ID, OWS_DOCTOR_WALLET_NONE, "Warn", "Minor", "Fix it"), ]; let report = DoctorReport::new(findings); assert_eq!(report.overall_status, DoctorStatus::Warning); @@ -282,8 +362,8 @@ mod tests { #[test] fn test_overall_status_all_skipped() { let findings = vec![ - DoctorFinding::skipped(ID, "Skipped", "Not applicable"), - DoctorFinding::skipped(ID, "Skipped 2", "Also not applicable"), + DoctorFinding::skipped(ID, OWS_DOCTOR_VAULT_MISSING, "Skipped", "Not applicable"), + DoctorFinding::skipped(ID, OWS_DOCTOR_CONFIG_MISSING, "Skipped 2", "Also not applicable"), ]; let report = DoctorReport::new(findings); assert_eq!(report.overall_status, DoctorStatus::Skipped); @@ -296,7 +376,7 @@ mod tests { fn test_overall_status_mixed_but_passing() { let findings = vec![ DoctorFinding::ok(ID, "Good", "All good"), - DoctorFinding::skipped(ID, "Skipped", "Not applicable"), + DoctorFinding::skipped(ID, OWS_DOCTOR_CONFIG_MISSING, "Skipped", "Not applicable"), ]; let report = DoctorReport::new(findings); assert_eq!(report.overall_status, DoctorStatus::Ok); @@ -309,9 +389,9 @@ mod tests { fn test_findings_with_status() { let findings = vec![ DoctorFinding::ok(ID, "Good", "All good"), - DoctorFinding::warning(ID, "Warn", "Minor", "Fix it"), - DoctorFinding::error(ID, "Bad", "Critical", "Fix it"), - DoctorFinding::skipped(ID, "Skipped", "Not applicable"), + DoctorFinding::warning(ID, OWS_DOCTOR_WALLET_NONE, "Warn", "Minor", "Fix it"), + DoctorFinding::error(ID, OWS_DOCTOR_VAULT_MISSING, "Bad", "Critical", "Fix it"), + DoctorFinding::skipped(ID, OWS_DOCTOR_CONFIG_MISSING, "Skipped", "Not applicable"), ]; let report = DoctorReport::new(findings); assert_eq!(report.findings_with_status(DoctorStatus::Ok).len(), 1); @@ -322,20 +402,25 @@ mod tests { #[test] fn test_finding_builder_with_path() { - let finding = DoctorFinding::ok(ID, "Title", "Detail") - .with_path(std::path::PathBuf::from("/test/path")); + let finding = + DoctorFinding::ok(ID, "Title", "Detail").with_path(std::path::PathBuf::from("/test/path")); assert!(finding.path.is_some()); - assert_eq!( - finding.path.unwrap(), - std::path::PathBuf::from("/test/path") - ); + assert_eq!(finding.path.unwrap(), std::path::PathBuf::from("/test/path")); + } + + #[test] + fn test_skipped_has_code() { + let finding = DoctorFinding::skipped(ID, OWS_DOCTOR_VAULT_MISSING, "Skipped", "Vault absent"); + assert_eq!(finding.code, Some(OWS_DOCTOR_VAULT_MISSING)); + assert_eq!(finding.status, DoctorStatus::Skipped); } #[test] - fn test_finding_builder_with_code() { + fn test_error_has_code() { let finding = - DoctorFinding::error(ID, "Title", "Detail", "Fix it").with_code("ERR_VAULT_MISSING"); - assert_eq!(finding.code, Some("ERR_VAULT_MISSING")); + DoctorFinding::error(ID, OWS_DOCTOR_VAULT_MISSING, "Title", "Detail", "Fix it"); + assert_eq!(finding.code, Some(OWS_DOCTOR_VAULT_MISSING)); + assert_eq!(finding.status, DoctorStatus::Error); } #[test] diff --git a/ows/crates/ows-cli/src/commands/doctor/vault_inspector.rs b/ows/crates/ows-cli/src/commands/doctor/vault_inspector.rs index 46a57c1e..6a3e1255 100644 --- a/ows/crates/ows-cli/src/commands/doctor/vault_inspector.rs +++ b/ows/crates/ows-cli/src/commands/doctor/vault_inspector.rs @@ -1,34 +1,18 @@ //! Read-only vault artifact inspection for `ows doctor`. -//! -//! Provides functions to enumerate and validate wallet, key, and policy files -//! without decrypting secrets or modifying any state. -//! -//! All functions return findings directly; they do not mutate or create files. -//! -//! # Validation approach -//! -//! - **Wallet files**: Deserialize as `EncryptedWallet`. The serde derivation -//! validates structure. We additionally check for empty ID and empty accounts. -//! - **Key files**: Deserialize as `ApiKeyFile`. Validates all required fields. -//! - **Policy files**: Deserialize as `Policy`. Validates structure. -//! -//! # Error taxonomy -//! -//! | Condition | Status | Code | -//! |-----------|--------|------| -//! | File unreadable (permissions) | Error | `ERR_FILE_UNREADABLE` | -//! | File not valid JSON | Error | `ERR_FILE_MALFORMED` | -//! | JSON parses but schema invalid | Error | `ERR_METADATA_INVALID` | -//! | No artifacts of this type | Skipped | — | -//! | All artifacts valid | Ok | — | -//! | Some artifacts valid, some invalid | Warning | `WARN_ARTIFACTS_CORRUPTED` | use std::fs; use std::path::Path; use ows_core::{ApiKeyFile, EncryptedWallet, Policy}; -use crate::commands::doctor::report::{DoctorCheckId, DoctorFinding}; +use crate::commands::doctor::report::{ + DoctorCheckId, DoctorFinding, OWS_DOCTOR_DIR_UNREADABLE, + OWS_DOCTOR_KEY_FILE_INVALID, OWS_DOCTOR_KEY_FILE_UNREADABLE, OWS_DOCTOR_KEY_NONE, + OWS_DOCTOR_KEY_SOME_CORRUPT, OWS_DOCTOR_POLICY_FILE_INVALID, + OWS_DOCTOR_POLICY_FILE_UNREADABLE, OWS_DOCTOR_POLICY_NONE, OWS_DOCTOR_POLICY_SOME_CORRUPT, + OWS_DOCTOR_WALLET_FILE_INVALID, OWS_DOCTOR_WALLET_FILE_UNREADABLE, + OWS_DOCTOR_WALLET_METADATA_CORRUPT, OWS_DOCTOR_WALLET_NONE, OWS_DOCTOR_WALLET_SOME_CORRUPT, +}; // --------------------------------------------------------------------------- // Check IDs @@ -45,15 +29,13 @@ pub const CHECK_POLICY_FILES: DoctorCheckId = DoctorCheckId::new("vault.policy_f /// Inspect all wallet files in the vault. /// /// Returns one finding per artifact, plus a summary finding. -/// -/// # Arguments -/// * `vault_path` - Path to the vault directory (e.g. `~/.ows`) pub fn check_wallet_files(vault_path: &Path) -> Vec { let wallets_dir = vault_path.join("wallets"); if !wallets_dir.exists() { return vec![DoctorFinding::skipped( CHECK_WALLET_FILES, + OWS_DOCTOR_DIR_UNREADABLE, "No wallets directory", "Wallets directory does not exist; skipping wallet inspection.", )]; @@ -64,12 +46,12 @@ pub fn check_wallet_files(vault_path: &Path) -> Vec { Err(e) => { return vec![DoctorFinding::error( CHECK_WALLET_FILES, + OWS_DOCTOR_DIR_UNREADABLE, "Cannot read wallets directory", - &format!("Wallets directory exists but cannot be read: {}", e), + &format!("Wallets directory exists but cannot be read: {}.", e), "Check directory permissions.", ) - .with_path(wallets_dir) - .with_code("ERR_DIR_UNREADABLE")]; + .with_path(wallets_dir)]; } }; @@ -82,12 +64,12 @@ pub fn check_wallet_files(vault_path: &Path) -> Vec { if json_entries.is_empty() { return vec![DoctorFinding::warning( CHECK_WALLET_FILES, - "No wallets present", - "No wallet files found in the wallets directory.", + OWS_DOCTOR_WALLET_NONE, + "No wallet files found", + "The wallets directory exists but contains no wallet files.", "Run `ows wallet create` to create your first wallet.", ) - .with_path(wallets_dir) - .with_code("WARN_NO_WALLETS")]; + .with_path(wallets_dir)]; } let mut findings = Vec::new(); @@ -103,16 +85,14 @@ pub fn check_wallet_files(vault_path: &Path) -> Vec { Ok(c) => c, Err(e) => { corrupted_count += 1; - findings.push( - DoctorFinding::error( - CHECK_WALLET_FILES, - "Wallet file unreadable", - &format!("{}: cannot read file: {}", file_name, e), - "Check file permissions.", - ) - .with_path(path) - .with_code("ERR_FILE_UNREADABLE"), - ); + findings.push(DoctorFinding::error( + CHECK_WALLET_FILES, + OWS_DOCTOR_WALLET_FILE_UNREADABLE, + "Wallet file cannot be read", + &format!("{}: I/O error reading file: {}.", file_name, e), + "Check file permissions with `ls -l ~/.ows/wallets/`.", + ) + .with_path(path)); continue; } }; @@ -124,62 +104,59 @@ pub fn check_wallet_files(vault_path: &Path) -> Vec { corrupted_count += 1; findings.push(DoctorFinding::error( CHECK_WALLET_FILES, - "Wallet file malformed", - &format!("{}: invalid JSON: {}", file_name, e), - "This wallet file is corrupted. Export the mnemonic (if possible) and recreate the wallet.", + OWS_DOCTOR_WALLET_FILE_INVALID, + "Wallet file is not valid JSON", + &format!( + "{}: JSON parse error. This file is corrupted: {}.", + file_name, e + ), + "Export the mnemonic (if possible) and recreate the wallet with `ows wallet create`.", ) - .with_path(path) - .with_code("ERR_FILE_MALFORMED")); + .with_path(path)); continue; } }; // Additional metadata validation if wallet.id.is_empty() { - findings.push( - DoctorFinding::error( - CHECK_WALLET_FILES, - "Wallet has empty ID", - &format!("{}: wallet ID field is empty", file_name), - "Recreate the wallet from the mnemonic.", - ) - .with_path(path) - .with_code("ERR_METADATA_INVALID"), - ); + findings.push(DoctorFinding::error( + CHECK_WALLET_FILES, + OWS_DOCTOR_WALLET_METADATA_CORRUPT, + "Wallet has an empty ID field", + &format!("{}: the wallet `id` field is empty.", file_name), + "Export the mnemonic and recreate the wallet with `ows wallet create`.", + ) + .with_path(path)); corrupted_count += 1; continue; } if wallet.created_at.is_empty() { - findings.push( - DoctorFinding::error( - CHECK_WALLET_FILES, - "Wallet has empty created_at", - &format!("{}: created_at field is empty", file_name), - "Recreate the wallet from the mnemonic.", - ) - .with_path(path) - .with_code("ERR_METADATA_INVALID"), - ); + findings.push(DoctorFinding::error( + CHECK_WALLET_FILES, + OWS_DOCTOR_WALLET_METADATA_CORRUPT, + "Wallet has an empty created_at field", + &format!("{}: the `created_at` field is empty.", file_name), + "Export the mnemonic and recreate the wallet with `ows wallet create`.", + ) + .with_path(path)); corrupted_count += 1; continue; } // Validate created_at is valid RFC3339 if chrono::DateTime::parse_from_rfc3339(&wallet.created_at).is_err() { - findings.push( - DoctorFinding::error( - CHECK_WALLET_FILES, - "Wallet has invalid created_at", - &format!( - "{}: created_at is not valid RFC3339: `{}`", - file_name, wallet.created_at - ), - "Recreate the wallet from the mnemonic.", - ) - .with_path(path) - .with_code("ERR_METADATA_INVALID"), - ); + findings.push(DoctorFinding::error( + CHECK_WALLET_FILES, + OWS_DOCTOR_WALLET_METADATA_CORRUPT, + "Wallet has an invalid created_at field", + &format!( + "{}: `created_at` is not valid RFC3339: `{}`.", + file_name, wallet.created_at + ), + "Export the mnemonic and recreate the wallet with `ows wallet create`.", + ) + .with_path(path)); corrupted_count += 1; continue; } @@ -187,27 +164,25 @@ pub fn check_wallet_files(vault_path: &Path) -> Vec { valid_count += 1; } - // Push summary finding + // Summary finding if corrupted_count == 0 { findings.push(DoctorFinding::ok( CHECK_WALLET_FILES, - "Wallet files valid", - &format!("All {} wallet file(s) are valid.", valid_count), + "All wallet files are valid", + &format!("{} wallet file(s) parsed successfully.", valid_count), )); } else { - findings.push( - DoctorFinding::warning( - CHECK_WALLET_FILES, - "Some wallet files corrupted", - &format!( - "{} of {} wallet file(s) are corrupted.", - corrupted_count, - valid_count + corrupted_count - ), - "Export valid wallets and recreate the corrupted ones.", - ) - .with_code("WARN_ARTIFACTS_CORRUPTED"), - ); + findings.push(DoctorFinding::warning( + CHECK_WALLET_FILES, + OWS_DOCTOR_WALLET_SOME_CORRUPT, + "Some wallet files are corrupted", + &format!( + "{} of {} wallet file(s) are corrupted.", + corrupted_count, + valid_count + corrupted_count + ), + "Export the mnemonic from any valid wallets and recreate the corrupted ones.", + )); } findings @@ -226,8 +201,9 @@ pub fn check_key_files(vault_path: &Path) -> Vec { if !keys_dir.exists() { return vec![DoctorFinding::skipped( CHECK_KEY_FILES, + OWS_DOCTOR_DIR_UNREADABLE, "No keys directory", - "Keys directory does not exist; skipping key file inspection.", + "Keys directory does not exist; skipping API key file inspection.", )]; } @@ -236,12 +212,12 @@ pub fn check_key_files(vault_path: &Path) -> Vec { Err(e) => { return vec![DoctorFinding::error( CHECK_KEY_FILES, + OWS_DOCTOR_DIR_UNREADABLE, "Cannot read keys directory", - &format!("Keys directory exists but cannot be read: {}", e), + &format!("Keys directory exists but cannot be read: {}.", e), "Check directory permissions.", ) - .with_path(keys_dir) - .with_code("ERR_DIR_UNREADABLE")]; + .with_path(keys_dir)]; } }; @@ -253,8 +229,9 @@ pub fn check_key_files(vault_path: &Path) -> Vec { if json_entries.is_empty() { return vec![DoctorFinding::skipped( CHECK_KEY_FILES, - "No API keys present", - "No key files found in the keys directory.", + OWS_DOCTOR_KEY_NONE, + "No API key files found", + "The keys directory is empty.", ) .with_path(keys_dir)]; } @@ -271,16 +248,14 @@ pub fn check_key_files(vault_path: &Path) -> Vec { Ok(c) => c, Err(e) => { corrupted_count += 1; - findings.push( - DoctorFinding::error( - CHECK_KEY_FILES, - "Key file unreadable", - &format!("{}: cannot read file: {}", file_name, e), - "Check file permissions.", - ) - .with_path(path) - .with_code("ERR_FILE_UNREADABLE"), - ); + findings.push(DoctorFinding::error( + CHECK_KEY_FILES, + OWS_DOCTOR_KEY_FILE_UNREADABLE, + "API key file cannot be read", + &format!("{}: I/O error reading file: {}.", file_name, e), + "Check file permissions with `ls -l ~/.ows/keys/`.", + ) + .with_path(path)); continue; } }; @@ -289,16 +264,14 @@ pub fn check_key_files(vault_path: &Path) -> Vec { Ok(k) => k, Err(e) => { corrupted_count += 1; - findings.push( - DoctorFinding::error( - CHECK_KEY_FILES, - "Key file malformed", - &format!("{}: invalid JSON: {}", file_name, e), - "Delete and recreate the API key.", - ) - .with_path(path) - .with_code("ERR_FILE_MALFORMED"), - ); + findings.push(DoctorFinding::error( + CHECK_KEY_FILES, + OWS_DOCTOR_KEY_FILE_INVALID, + "API key file is not valid JSON", + &format!("{}: JSON parse error. This file is corrupted: {}.", file_name, e), + "Delete and recreate the API key with `ows key revoke` then `ows key create`.", + ) + .with_path(path)); continue; } }; @@ -309,23 +282,21 @@ pub fn check_key_files(vault_path: &Path) -> Vec { if corrupted_count == 0 { findings.push(DoctorFinding::ok( CHECK_KEY_FILES, - "Key files valid", - &format!("All {} key file(s) are valid.", valid_count), + "All API key files are valid", + &format!("{} API key file(s) parsed successfully.", valid_count), )); } else { - findings.push( - DoctorFinding::warning( - CHECK_KEY_FILES, - "Some key files corrupted", - &format!( - "{} of {} key file(s) are corrupted.", - corrupted_count, - valid_count + corrupted_count - ), - "Delete and recreate the corrupted API keys.", - ) - .with_code("WARN_ARTIFACTS_CORRUPTED"), - ); + findings.push(DoctorFinding::warning( + CHECK_KEY_FILES, + OWS_DOCTOR_KEY_SOME_CORRUPT, + "Some API key files are corrupted", + &format!( + "{} of {} API key file(s) are corrupted.", + corrupted_count, + valid_count + corrupted_count + ), + "Delete and recreate the corrupted API keys.", + )); } findings @@ -344,6 +315,7 @@ pub fn check_policy_files(vault_path: &Path) -> Vec { if !policies_dir.exists() { return vec![DoctorFinding::skipped( CHECK_POLICY_FILES, + OWS_DOCTOR_DIR_UNREADABLE, "No policies directory", "Policies directory does not exist; skipping policy file inspection.", )]; @@ -354,12 +326,12 @@ pub fn check_policy_files(vault_path: &Path) -> Vec { Err(e) => { return vec![DoctorFinding::error( CHECK_POLICY_FILES, + OWS_DOCTOR_DIR_UNREADABLE, "Cannot read policies directory", - &format!("Policies directory exists but cannot be read: {}", e), + &format!("Policies directory exists but cannot be read: {}.", e), "Check directory permissions.", ) - .with_path(policies_dir) - .with_code("ERR_DIR_UNREADABLE")]; + .with_path(policies_dir)]; } }; @@ -371,8 +343,9 @@ pub fn check_policy_files(vault_path: &Path) -> Vec { if json_entries.is_empty() { return vec![DoctorFinding::skipped( CHECK_POLICY_FILES, - "No policies present", - "No policy files found in the policies directory.", + OWS_DOCTOR_POLICY_NONE, + "No policy files found", + "The policies directory is empty.", ) .with_path(policies_dir)]; } @@ -389,16 +362,14 @@ pub fn check_policy_files(vault_path: &Path) -> Vec { Ok(c) => c, Err(e) => { corrupted_count += 1; - findings.push( - DoctorFinding::error( - CHECK_POLICY_FILES, - "Policy file unreadable", - &format!("{}: cannot read file: {}", file_name, e), - "Check file permissions.", - ) - .with_path(path) - .with_code("ERR_FILE_UNREADABLE"), - ); + findings.push(DoctorFinding::error( + CHECK_POLICY_FILES, + OWS_DOCTOR_POLICY_FILE_UNREADABLE, + "Policy file cannot be read", + &format!("{}: I/O error reading file: {}.", file_name, e), + "Check file permissions with `ls -l ~/.ows/policies/`.", + ) + .with_path(path)); continue; } }; @@ -407,16 +378,14 @@ pub fn check_policy_files(vault_path: &Path) -> Vec { Ok(p) => p, Err(e) => { corrupted_count += 1; - findings.push( - DoctorFinding::error( - CHECK_POLICY_FILES, - "Policy file malformed", - &format!("{}: invalid JSON: {}", file_name, e), - "Delete and recreate the policy.", - ) - .with_path(path) - .with_code("ERR_FILE_MALFORMED"), - ); + findings.push(DoctorFinding::error( + CHECK_POLICY_FILES, + OWS_DOCTOR_POLICY_FILE_INVALID, + "Policy file is not valid JSON", + &format!("{}: JSON parse error. This file is corrupted: {}.", file_name, e), + "Recreate the policy with `ows policy create`.", + ) + .with_path(path)); continue; } }; @@ -427,23 +396,21 @@ pub fn check_policy_files(vault_path: &Path) -> Vec { if corrupted_count == 0 { findings.push(DoctorFinding::ok( CHECK_POLICY_FILES, - "Policy files valid", - &format!("All {} policy file(s) are valid.", valid_count), + "All policy files are valid", + &format!("{} policy file(s) parsed successfully.", valid_count), )); } else { - findings.push( - DoctorFinding::warning( - CHECK_POLICY_FILES, - "Some policy files corrupted", - &format!( - "{} of {} policy file(s) are corrupted.", - corrupted_count, - valid_count + corrupted_count - ), - "Delete and recreate the corrupted policies.", - ) - .with_code("WARN_ARTIFACTS_CORRUPTED"), - ); + findings.push(DoctorFinding::warning( + CHECK_POLICY_FILES, + OWS_DOCTOR_POLICY_SOME_CORRUPT, + "Some policy files are corrupted", + &format!( + "{} of {} policy file(s) are corrupted.", + corrupted_count, + valid_count + corrupted_count + ), + "Recreate the corrupted policies.", + )); } findings @@ -499,6 +466,7 @@ mod tests { let findings = check_wallet_files(temp.path()); assert_eq!(findings.len(), 1); assert_eq!(findings[0].status, DoctorStatus::Skipped); + assert_eq!(findings[0].code, Some(OWS_DOCTOR_DIR_UNREADABLE)); } #[test] @@ -509,7 +477,7 @@ mod tests { let findings = check_wallet_files(&vault); assert_eq!(findings.len(), 1); assert_eq!(findings[0].status, DoctorStatus::Warning); - assert_eq!(findings[0].code, Some("WARN_NO_WALLETS")); + assert_eq!(findings[0].code, Some(OWS_DOCTOR_WALLET_NONE)); } #[test] @@ -541,9 +509,9 @@ mod tests { std::fs::write(wallets_dir.join("bad.json"), "{ invalid json }").ok(); let findings = check_wallet_files(&vault); - assert!(findings - .iter() - .any(|f| f.status == DoctorStatus::Error && f.code == Some("ERR_FILE_MALFORMED"))); + assert!(findings.iter().any(|f| { + f.status == DoctorStatus::Error && f.code == Some(OWS_DOCTOR_WALLET_FILE_INVALID) + })); } #[test] @@ -559,12 +527,11 @@ mod tests { let findings = check_wallet_files(&vault); assert!(findings .iter() - .any(|f| f.code == Some("ERR_METADATA_INVALID"))); + .any(|f| f.code == Some(OWS_DOCTOR_WALLET_METADATA_CORRUPT))); } #[test] fn test_wallet_files_invalid_created_at() { - // Create a wallet JSON with an invalid created_at RFC3339 string let temp = TempDir::new().unwrap(); let vault = temp.path().join(".ows"); let wallets_dir = vault.join("wallets"); @@ -586,10 +553,9 @@ mod tests { .ok(); let findings = check_wallet_files(&vault); - // Should detect invalid created_at assert!(findings .iter() - .any(|f| f.code == Some("ERR_METADATA_INVALID"))); + .any(|f| f.code == Some(OWS_DOCTOR_WALLET_METADATA_CORRUPT))); } #[test] @@ -608,11 +574,10 @@ mod tests { std::fs::write(wallets_dir.join("bad.json"), "{ bad }").ok(); let findings = check_wallet_files(&vault); - // With mixed valid and corrupted: one Error (malformed), one Warning (summary) assert!(findings.iter().any(|f| f.status == DoctorStatus::Error)); assert!(findings .iter() - .any(|f| f.code == Some("WARN_ARTIFACTS_CORRUPTED"))); + .any(|f| f.code == Some(OWS_DOCTOR_WALLET_SOME_CORRUPT))); } // ---- Key file tests ---- @@ -623,16 +588,18 @@ mod tests { let findings = check_key_files(temp.path()); assert_eq!(findings.len(), 1); assert_eq!(findings[0].status, DoctorStatus::Skipped); + assert_eq!(findings[0].code, Some(OWS_DOCTOR_DIR_UNREADABLE)); } #[test] fn test_key_files_empty_dir_is_skipped() { let temp = TempDir::new().unwrap(); let vault = temp.path().join(".ows"); - std::fs::create_dir(vault.join("keys")).ok(); + std::fs::create_dir_all(vault.join("keys")).ok(); let findings = check_key_files(&vault); assert_eq!(findings.len(), 1); assert_eq!(findings[0].status, DoctorStatus::Skipped); + assert_eq!(findings[0].code, Some(OWS_DOCTOR_KEY_NONE)); } #[test] @@ -658,9 +625,9 @@ mod tests { std::fs::write(keys_dir.join("bad.json"), "{ invalid }").ok(); let findings = check_key_files(&vault); - assert!(findings - .iter() - .any(|f| f.status == DoctorStatus::Error && f.code == Some("ERR_FILE_MALFORMED"))); + assert!(findings.iter().any(|f| { + f.status == DoctorStatus::Error && f.code == Some(OWS_DOCTOR_KEY_FILE_INVALID) + })); } // ---- Policy file tests ---- @@ -671,16 +638,18 @@ mod tests { let findings = check_policy_files(temp.path()); assert_eq!(findings.len(), 1); assert_eq!(findings[0].status, DoctorStatus::Skipped); + assert_eq!(findings[0].code, Some(OWS_DOCTOR_DIR_UNREADABLE)); } #[test] fn test_policy_files_empty_dir_is_skipped() { let temp = TempDir::new().unwrap(); let vault = temp.path().join(".ows"); - std::fs::create_dir(vault.join("policies")).ok(); + std::fs::create_dir_all(vault.join("policies")).ok(); let findings = check_policy_files(&vault); assert_eq!(findings.len(), 1); assert_eq!(findings[0].status, DoctorStatus::Skipped); + assert_eq!(findings[0].code, Some(OWS_DOCTOR_POLICY_NONE)); } #[test] @@ -706,8 +675,8 @@ mod tests { std::fs::write(policies_dir.join("bad.json"), "{ invalid }").ok(); let findings = check_policy_files(&vault); - assert!(findings - .iter() - .any(|f| f.status == DoctorStatus::Error && f.code == Some("ERR_FILE_MALFORMED"))); + assert!(findings.iter().any(|f| { + f.status == DoctorStatus::Error && f.code == Some(OWS_DOCTOR_POLICY_FILE_INVALID) + })); } } From 799b5b2e7169bc2a3c147d44933877d61225357e Mon Sep 17 00:00:00 2001 From: OKWN Date: Thu, 2 Apr 2026 13:41:08 +0300 Subject: [PATCH 6/9] docs: add ows doctor to CLI reference and update sdk-cli.md docs - Add `ows doctor` to cli-reference.md partial (source-of-truth) - Run readme/generate.sh to propagate to all generated READMEs - Update docs/sdk-cli.md with Phase 5 examples: - Add healthy vault sample output - Add missing vault sample output - Add corrupted wallet file example - Add platform caveats for permission checks (Unix-only) - Document exit code semantics Co-Authored-By: Claude Opus 4.6 --- bindings/node/README.md | 1 + docs/sdk-cli.md | 63 ++++++++++++++++++++++++++------ ows/README.md | 1 + ows/crates/ows-cli/README.md | 1 + ows/crates/ows-core/README.md | 1 + ows/crates/ows-lib/README.md | 1 + ows/crates/ows-signer/README.md | 1 + readme/partials/cli-reference.md | 3 +- 8 files changed, 59 insertions(+), 13 deletions(-) diff --git a/bindings/node/README.md b/bindings/node/README.md index 6483d9fb..4e62e3d2 100644 --- a/bindings/node/README.md +++ b/bindings/node/README.md @@ -84,6 +84,7 @@ ows sign tx --wallet agent-treasury --chain evm --tx "deadbeef..." | `ows key revoke` | Revoke an API key | | `ows update` | Update ows and bindings | | `ows uninstall` | Remove ows from the system | +| `ows doctor` | Run diagnostic checks on the OWS installation | ## Architecture diff --git a/docs/sdk-cli.md b/docs/sdk-cli.md index c232b3ea..08eb6d43 100644 --- a/docs/sdk-cli.md +++ b/docs/sdk-cli.md @@ -387,15 +387,18 @@ Run diagnostic checks on the OWS installation. `ows doctor` is a read-only comma - Vault and subdirectory existence - Config file validity (if present) - Wallet, key, and policy file integrity -- File permissions (Unix only; skipped on Windows) +- File permissions (Unix only; skipped on Windows/macOS) -Exit code is 0 when all checks pass or only warnings are found, and non-zero when errors are detected. +**Exit code semantics:** +- `0` — all checks passed, or only warnings were found (no errors) +- non-zero — at least one check produced an error ```bash ows doctor ``` -Sample output: +**Example: healthy vault** + ``` ============================================================ OWS Doctor @@ -403,26 +406,62 @@ Sample output: Vault path: ~/.ows - Errors: + Passed: ---------------------------------------- - ✗ Wallet file malformed: wallet-1.json: invalid JSON: ... - → Export the mnemonic (if possible) and recreate the wallet. + ✓ Vault path resolved + ✓ Vault exists + ✓ Logs directory present + ✓ Config file valid + + 4 passed 0 warnings 0 errors 6 skipped + + Result: OK — all checks passed +``` - Warnings: +**Example: missing vault (first run)** + +``` +============================================================ + OWS Doctor +============================================================ + + Vault path: ~/.ows + + Errors: ---------------------------------------- - ⚠ No wallets present: No wallet files found in the wallets directory. + ✗ Vault directory not found: No vault found at `~/.ows`. No wallets have been created yet. → Run `ows wallet create` to create your first wallet. - Passed: + Skipped: ---------------------------------------- - ✓ Vault path resolved - ✓ Config valid + ○ Logs directory check skipped + ○ Config file not present + ○ No wallets directory + ○ No keys directory + ○ No policies directory + ○ Permissions check skipped - 2 passed 1 warnings 1 errors 0 skipped + 1 passed 0 warnings 1 errors 6 skipped Result: FAILED — errors found ``` +**Example: corrupted wallet file** + +``` +Errors: + ---------------------------------------- + ✗ Wallet file is not valid JSON: wallet-1.json: JSON parse error. This file is corrupted: ... + → Export the mnemonic (if possible) and recreate the wallet with `ows wallet create`. + +Warnings: + ---------------------------------------- + ⚠ No wallet files found: The wallets directory exists but contains no wallet files. + → Run `ows wallet create` to create your first wallet. +``` + +**Platform caveats:** Permission checks (`vault.permissions`) only run on Unix/Linux systems. On Windows and macOS the check is reported as skipped with the note "Permission checks are Unix-only." + ## File Layout ``` diff --git a/ows/README.md b/ows/README.md index dcc8f228..0b8fd671 100644 --- a/ows/README.md +++ b/ows/README.md @@ -42,6 +42,7 @@ cargo build --workspace --release | `ows key revoke` | Revoke an API key | | `ows update` | Update ows and bindings | | `ows uninstall` | Remove ows from the system | +| `ows doctor` | Run diagnostic checks on the OWS installation | ## Language Bindings diff --git a/ows/crates/ows-cli/README.md b/ows/crates/ows-cli/README.md index dcc8f228..0b8fd671 100644 --- a/ows/crates/ows-cli/README.md +++ b/ows/crates/ows-cli/README.md @@ -42,6 +42,7 @@ cargo build --workspace --release | `ows key revoke` | Revoke an API key | | `ows update` | Update ows and bindings | | `ows uninstall` | Remove ows from the system | +| `ows doctor` | Run diagnostic checks on the OWS installation | ## Language Bindings diff --git a/ows/crates/ows-core/README.md b/ows/crates/ows-core/README.md index dcc8f228..0b8fd671 100644 --- a/ows/crates/ows-core/README.md +++ b/ows/crates/ows-core/README.md @@ -42,6 +42,7 @@ cargo build --workspace --release | `ows key revoke` | Revoke an API key | | `ows update` | Update ows and bindings | | `ows uninstall` | Remove ows from the system | +| `ows doctor` | Run diagnostic checks on the OWS installation | ## Language Bindings diff --git a/ows/crates/ows-lib/README.md b/ows/crates/ows-lib/README.md index dcc8f228..0b8fd671 100644 --- a/ows/crates/ows-lib/README.md +++ b/ows/crates/ows-lib/README.md @@ -42,6 +42,7 @@ cargo build --workspace --release | `ows key revoke` | Revoke an API key | | `ows update` | Update ows and bindings | | `ows uninstall` | Remove ows from the system | +| `ows doctor` | Run diagnostic checks on the OWS installation | ## Language Bindings diff --git a/ows/crates/ows-signer/README.md b/ows/crates/ows-signer/README.md index dcc8f228..0b8fd671 100644 --- a/ows/crates/ows-signer/README.md +++ b/ows/crates/ows-signer/README.md @@ -42,6 +42,7 @@ cargo build --workspace --release | `ows key revoke` | Revoke an API key | | `ows update` | Update ows and bindings | | `ows uninstall` | Remove ows from the system | +| `ows doctor` | Run diagnostic checks on the OWS installation | ## Language Bindings diff --git a/readme/partials/cli-reference.md b/readme/partials/cli-reference.md index d2823da8..39035bed 100644 --- a/readme/partials/cli-reference.md +++ b/readme/partials/cli-reference.md @@ -19,4 +19,5 @@ | `ows key list` | List all API keys | | `ows key revoke` | Revoke an API key | | `ows update` | Update ows and bindings | -| `ows uninstall` | Remove ows from the system | \ No newline at end of file +| `ows uninstall` | Remove ows from the system | +| `ows doctor` | Run diagnostic checks on the OWS installation | \ No newline at end of file From 1bbe16210b873c7628371b5927e7b7dfa98f42e5 Mon Sep 17 00:00:00 2001 From: OKWN Date: Thu, 2 Apr 2026 13:46:02 +0300 Subject: [PATCH 7/9] fix formatting in doctor module Apply `cargo fmt` to the doctor module. Co-Authored-By: Claude Opus 4.6 --- .../ows-cli/src/commands/doctor/checks.rs | 4 +- .../ows-cli/src/commands/doctor/report.rs | 19 ++- .../src/commands/doctor/vault_inspector.rs | 147 ++++++++++-------- 3 files changed, 98 insertions(+), 72 deletions(-) diff --git a/ows/crates/ows-cli/src/commands/doctor/checks.rs b/ows/crates/ows-cli/src/commands/doctor/checks.rs index a78bc231..2b0512a8 100644 --- a/ows/crates/ows-cli/src/commands/doctor/checks.rs +++ b/ows/crates/ows-cli/src/commands/doctor/checks.rs @@ -168,8 +168,8 @@ pub fn check_config() -> Vec { #[cfg(unix)] pub fn check_vault_permissions() -> Vec { use crate::commands::doctor::report::{ - OWS_DOCTOR_PERM_VAULT_INSECURE, OWS_DOCTOR_PERM_WALLET_FILE_INSECURE, - OWS_DOCTOR_PERM_WALLETS_INSECURE, + OWS_DOCTOR_PERM_VAULT_INSECURE, OWS_DOCTOR_PERM_WALLETS_INSECURE, + OWS_DOCTOR_PERM_WALLET_FILE_INSECURE, }; use std::os::unix::fs::PermissionsExt; diff --git a/ows/crates/ows-cli/src/commands/doctor/report.rs b/ows/crates/ows-cli/src/commands/doctor/report.rs index d2d7f388..54d9f70c 100644 --- a/ows/crates/ows-cli/src/commands/doctor/report.rs +++ b/ows/crates/ows-cli/src/commands/doctor/report.rs @@ -363,7 +363,12 @@ mod tests { fn test_overall_status_all_skipped() { let findings = vec![ DoctorFinding::skipped(ID, OWS_DOCTOR_VAULT_MISSING, "Skipped", "Not applicable"), - DoctorFinding::skipped(ID, OWS_DOCTOR_CONFIG_MISSING, "Skipped 2", "Also not applicable"), + DoctorFinding::skipped( + ID, + OWS_DOCTOR_CONFIG_MISSING, + "Skipped 2", + "Also not applicable", + ), ]; let report = DoctorReport::new(findings); assert_eq!(report.overall_status, DoctorStatus::Skipped); @@ -402,15 +407,19 @@ mod tests { #[test] fn test_finding_builder_with_path() { - let finding = - DoctorFinding::ok(ID, "Title", "Detail").with_path(std::path::PathBuf::from("/test/path")); + let finding = DoctorFinding::ok(ID, "Title", "Detail") + .with_path(std::path::PathBuf::from("/test/path")); assert!(finding.path.is_some()); - assert_eq!(finding.path.unwrap(), std::path::PathBuf::from("/test/path")); + assert_eq!( + finding.path.unwrap(), + std::path::PathBuf::from("/test/path") + ); } #[test] fn test_skipped_has_code() { - let finding = DoctorFinding::skipped(ID, OWS_DOCTOR_VAULT_MISSING, "Skipped", "Vault absent"); + let finding = + DoctorFinding::skipped(ID, OWS_DOCTOR_VAULT_MISSING, "Skipped", "Vault absent"); assert_eq!(finding.code, Some(OWS_DOCTOR_VAULT_MISSING)); assert_eq!(finding.status, DoctorStatus::Skipped); } diff --git a/ows/crates/ows-cli/src/commands/doctor/vault_inspector.rs b/ows/crates/ows-cli/src/commands/doctor/vault_inspector.rs index 6a3e1255..a47dabae 100644 --- a/ows/crates/ows-cli/src/commands/doctor/vault_inspector.rs +++ b/ows/crates/ows-cli/src/commands/doctor/vault_inspector.rs @@ -6,12 +6,12 @@ use std::path::Path; use ows_core::{ApiKeyFile, EncryptedWallet, Policy}; use crate::commands::doctor::report::{ - DoctorCheckId, DoctorFinding, OWS_DOCTOR_DIR_UNREADABLE, - OWS_DOCTOR_KEY_FILE_INVALID, OWS_DOCTOR_KEY_FILE_UNREADABLE, OWS_DOCTOR_KEY_NONE, - OWS_DOCTOR_KEY_SOME_CORRUPT, OWS_DOCTOR_POLICY_FILE_INVALID, - OWS_DOCTOR_POLICY_FILE_UNREADABLE, OWS_DOCTOR_POLICY_NONE, OWS_DOCTOR_POLICY_SOME_CORRUPT, - OWS_DOCTOR_WALLET_FILE_INVALID, OWS_DOCTOR_WALLET_FILE_UNREADABLE, - OWS_DOCTOR_WALLET_METADATA_CORRUPT, OWS_DOCTOR_WALLET_NONE, OWS_DOCTOR_WALLET_SOME_CORRUPT, + DoctorCheckId, DoctorFinding, OWS_DOCTOR_DIR_UNREADABLE, OWS_DOCTOR_KEY_FILE_INVALID, + OWS_DOCTOR_KEY_FILE_UNREADABLE, OWS_DOCTOR_KEY_NONE, OWS_DOCTOR_KEY_SOME_CORRUPT, + OWS_DOCTOR_POLICY_FILE_INVALID, OWS_DOCTOR_POLICY_FILE_UNREADABLE, OWS_DOCTOR_POLICY_NONE, + OWS_DOCTOR_POLICY_SOME_CORRUPT, OWS_DOCTOR_WALLET_FILE_INVALID, + OWS_DOCTOR_WALLET_FILE_UNREADABLE, OWS_DOCTOR_WALLET_METADATA_CORRUPT, OWS_DOCTOR_WALLET_NONE, + OWS_DOCTOR_WALLET_SOME_CORRUPT, }; // --------------------------------------------------------------------------- @@ -85,14 +85,16 @@ pub fn check_wallet_files(vault_path: &Path) -> Vec { Ok(c) => c, Err(e) => { corrupted_count += 1; - findings.push(DoctorFinding::error( - CHECK_WALLET_FILES, - OWS_DOCTOR_WALLET_FILE_UNREADABLE, - "Wallet file cannot be read", - &format!("{}: I/O error reading file: {}.", file_name, e), - "Check file permissions with `ls -l ~/.ows/wallets/`.", - ) - .with_path(path)); + findings.push( + DoctorFinding::error( + CHECK_WALLET_FILES, + OWS_DOCTOR_WALLET_FILE_UNREADABLE, + "Wallet file cannot be read", + &format!("{}: I/O error reading file: {}.", file_name, e), + "Check file permissions with `ls -l ~/.ows/wallets/`.", + ) + .with_path(path), + ); continue; } }; @@ -119,44 +121,50 @@ pub fn check_wallet_files(vault_path: &Path) -> Vec { // Additional metadata validation if wallet.id.is_empty() { - findings.push(DoctorFinding::error( - CHECK_WALLET_FILES, - OWS_DOCTOR_WALLET_METADATA_CORRUPT, - "Wallet has an empty ID field", - &format!("{}: the wallet `id` field is empty.", file_name), - "Export the mnemonic and recreate the wallet with `ows wallet create`.", - ) - .with_path(path)); + findings.push( + DoctorFinding::error( + CHECK_WALLET_FILES, + OWS_DOCTOR_WALLET_METADATA_CORRUPT, + "Wallet has an empty ID field", + &format!("{}: the wallet `id` field is empty.", file_name), + "Export the mnemonic and recreate the wallet with `ows wallet create`.", + ) + .with_path(path), + ); corrupted_count += 1; continue; } if wallet.created_at.is_empty() { - findings.push(DoctorFinding::error( - CHECK_WALLET_FILES, - OWS_DOCTOR_WALLET_METADATA_CORRUPT, - "Wallet has an empty created_at field", - &format!("{}: the `created_at` field is empty.", file_name), - "Export the mnemonic and recreate the wallet with `ows wallet create`.", - ) - .with_path(path)); + findings.push( + DoctorFinding::error( + CHECK_WALLET_FILES, + OWS_DOCTOR_WALLET_METADATA_CORRUPT, + "Wallet has an empty created_at field", + &format!("{}: the `created_at` field is empty.", file_name), + "Export the mnemonic and recreate the wallet with `ows wallet create`.", + ) + .with_path(path), + ); corrupted_count += 1; continue; } // Validate created_at is valid RFC3339 if chrono::DateTime::parse_from_rfc3339(&wallet.created_at).is_err() { - findings.push(DoctorFinding::error( - CHECK_WALLET_FILES, - OWS_DOCTOR_WALLET_METADATA_CORRUPT, - "Wallet has an invalid created_at field", - &format!( - "{}: `created_at` is not valid RFC3339: `{}`.", - file_name, wallet.created_at - ), - "Export the mnemonic and recreate the wallet with `ows wallet create`.", - ) - .with_path(path)); + findings.push( + DoctorFinding::error( + CHECK_WALLET_FILES, + OWS_DOCTOR_WALLET_METADATA_CORRUPT, + "Wallet has an invalid created_at field", + &format!( + "{}: `created_at` is not valid RFC3339: `{}`.", + file_name, wallet.created_at + ), + "Export the mnemonic and recreate the wallet with `ows wallet create`.", + ) + .with_path(path), + ); corrupted_count += 1; continue; } @@ -248,14 +256,16 @@ pub fn check_key_files(vault_path: &Path) -> Vec { Ok(c) => c, Err(e) => { corrupted_count += 1; - findings.push(DoctorFinding::error( - CHECK_KEY_FILES, - OWS_DOCTOR_KEY_FILE_UNREADABLE, - "API key file cannot be read", - &format!("{}: I/O error reading file: {}.", file_name, e), - "Check file permissions with `ls -l ~/.ows/keys/`.", - ) - .with_path(path)); + findings.push( + DoctorFinding::error( + CHECK_KEY_FILES, + OWS_DOCTOR_KEY_FILE_UNREADABLE, + "API key file cannot be read", + &format!("{}: I/O error reading file: {}.", file_name, e), + "Check file permissions with `ls -l ~/.ows/keys/`.", + ) + .with_path(path), + ); continue; } }; @@ -362,14 +372,16 @@ pub fn check_policy_files(vault_path: &Path) -> Vec { Ok(c) => c, Err(e) => { corrupted_count += 1; - findings.push(DoctorFinding::error( - CHECK_POLICY_FILES, - OWS_DOCTOR_POLICY_FILE_UNREADABLE, - "Policy file cannot be read", - &format!("{}: I/O error reading file: {}.", file_name, e), - "Check file permissions with `ls -l ~/.ows/policies/`.", - ) - .with_path(path)); + findings.push( + DoctorFinding::error( + CHECK_POLICY_FILES, + OWS_DOCTOR_POLICY_FILE_UNREADABLE, + "Policy file cannot be read", + &format!("{}: I/O error reading file: {}.", file_name, e), + "Check file permissions with `ls -l ~/.ows/policies/`.", + ) + .with_path(path), + ); continue; } }; @@ -378,14 +390,19 @@ pub fn check_policy_files(vault_path: &Path) -> Vec { Ok(p) => p, Err(e) => { corrupted_count += 1; - findings.push(DoctorFinding::error( - CHECK_POLICY_FILES, - OWS_DOCTOR_POLICY_FILE_INVALID, - "Policy file is not valid JSON", - &format!("{}: JSON parse error. This file is corrupted: {}.", file_name, e), - "Recreate the policy with `ows policy create`.", - ) - .with_path(path)); + findings.push( + DoctorFinding::error( + CHECK_POLICY_FILES, + OWS_DOCTOR_POLICY_FILE_INVALID, + "Policy file is not valid JSON", + &format!( + "{}: JSON parse error. This file is corrupted: {}.", + file_name, e + ), + "Recreate the policy with `ows policy create`.", + ) + .with_path(path), + ); continue; } }; From c5fa6ef9ebab0dd42beb7cbba0d3dc0e3f211f79 Mon Sep 17 00:00:00 2001 From: OKWN Date: Thu, 2 Apr 2026 20:19:27 +0300 Subject: [PATCH 8/9] feat(ows-cli): add 'ows doctor' diagnostic command --- docs/sdk-cli.md | 6 +++--- ows/crates/ows-cli/src/commands/doctor/report.rs | 4 ---- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/docs/sdk-cli.md b/docs/sdk-cli.md index 08eb6d43..23a629ff 100644 --- a/docs/sdk-cli.md +++ b/docs/sdk-cli.md @@ -451,13 +451,13 @@ ows doctor ``` Errors: ---------------------------------------- - ✗ Wallet file is not valid JSON: wallet-1.json: JSON parse error. This file is corrupted: ... + ✗ Wallet file is not valid JSON: bad.json: JSON parse error. This file is corrupted: key must be a string at line 1 column 3. → Export the mnemonic (if possible) and recreate the wallet with `ows wallet create`. Warnings: ---------------------------------------- - ⚠ No wallet files found: The wallets directory exists but contains no wallet files. - → Run `ows wallet create` to create your first wallet. + ⚠ Some wallet files are corrupted: 1 of 2 wallet file(s) are corrupted. + → Export the mnemonic from any valid wallets and recreate the corrupted ones. ``` **Platform caveats:** Permission checks (`vault.permissions`) only run on Unix/Linux systems. On Windows and macOS the check is reported as skipped with the note "Permission checks are Unix-only." diff --git a/ows/crates/ows-cli/src/commands/doctor/report.rs b/ows/crates/ows-cli/src/commands/doctor/report.rs index 54d9f70c..04f46efc 100644 --- a/ows/crates/ows-cli/src/commands/doctor/report.rs +++ b/ows/crates/ows-cli/src/commands/doctor/report.rs @@ -221,10 +221,6 @@ impl DoctorSummary { pub fn total(&self) -> usize { self.ok + self.warnings + self.errors + self.skipped } - - pub fn has_failures(&self) -> bool { - self.errors > 0 - } } /// Aggregated diagnostic report from all checks. From bf8ef83b6ff9dbf58e949b4b82f3d449a52ad3d5 Mon Sep 17 00:00:00 2001 From: OKWN Date: Thu, 2 Apr 2026 20:39:17 +0300 Subject: [PATCH 9/9] fix(ows-cli): correct HOME remediation in doctor output --- .../ows-cli/src/commands/doctor/checks.rs | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/ows/crates/ows-cli/src/commands/doctor/checks.rs b/ows/crates/ows-cli/src/commands/doctor/checks.rs index 2b0512a8..b05b42cc 100644 --- a/ows/crates/ows-cli/src/commands/doctor/checks.rs +++ b/ows/crates/ows-cli/src/commands/doctor/checks.rs @@ -52,7 +52,7 @@ pub fn check_vault_path() -> Vec { OWS_DOCTOR_ENV_HOME_NOT_SET, "HOME environment variable is not set", "Vault path resolution may be unreliable without HOME. Set HOME to your user directory.", - "Set the HOME environment variable (e.g. HOME=~/.ows in your shell profile).", + "Set HOME to your user home directory (e.g. HOME=/home/alice). OWS derives the vault path as $HOME/.ows.", )); } @@ -478,4 +478,29 @@ mod tests { let findings = vault_inspector::check_key_files(&vault); assert!(findings.iter().any(|f| f.status == DoctorStatus::Ok)); } + + #[test] + fn test_home_not_set_remediation_text() { + // Regression: HOME not-set remediation must not suggest HOME=~/.ows + // (that would nest the vault path as $HOME/.ows/.ows) + std::env::remove_var("HOME"); + let findings = check_vault_path(); + let home_error = findings + .iter() + .find(|f| f.code == Some(OWS_DOCTOR_ENV_HOME_NOT_SET)) + .expect("HOME not-set must produce a finding"); + + assert!( + !home_error.suggestion.as_ref().unwrap().contains("~/.ows"), + "HOME remediation must not suggest HOME=~/.ows" + ); + assert!( + home_error + .suggestion + .as_ref() + .unwrap() + .contains("$HOME/.ows"), + "HOME remediation must explain vault is derived as $HOME/.ows" + ); + } }