From 4f68a0e50ddeccf0de8e4153465a8419db59c2fa Mon Sep 17 00:00:00 2001 From: "wallydz-bot[bot]" <2909976+wallydz-bot[bot]@users.noreply.github.com> Date: Sat, 21 Feb 2026 19:25:49 +0100 Subject: [PATCH] feat(architecture): Harden state management and WireGuard config generation --- AI_REPORTS/architecture.md | 95 ++++++++++++++++++++++++++++++++++++++ src-tauri/src/config.rs | 22 ++------- src/stores/index.ts | 20 ++++++-- 3 files changed, 115 insertions(+), 22 deletions(-) create mode 100644 AI_REPORTS/architecture.md diff --git a/AI_REPORTS/architecture.md b/AI_REPORTS/architecture.md new file mode 100644 index 0000000..69a78ac --- /dev/null +++ b/AI_REPORTS/architecture.md @@ -0,0 +1,95 @@ +# VPNht Architecture & Tech-Debt Review + +## Overview +This report identifies high-risk tech debt in the **VPNht Tauri rewrite** and proposes targeted improvements. + +--- + +## 🔍 Review Summary +### **State Management** +- **✅ Good**: Zustand + Immer for immutable state. +- **⚠️ Issues**: Race conditions, no state validation, latency map mutability. + +### **IPC Boundaries** +- **✅ Good**: `Arc>` for thread safety. +- **⚠️ Issues**: No `Mutex` timeout, no input validation, mock data in production. + +### **Configuration Management** +- **✅ Good**: WireGuard config generation and parsing. +- **⚠️ Issues**: Placeholder keys, hardcoded values, no runtime reloading. + +### **Error Handling & Logging** +- **✅ Good**: Structured logging with `tracing`. +- **⚠️ Issues**: Errors converted to `String`, no sensitive data redaction. + +### **Killswitch & Network Safety** +- **✅ Good**: Linux `iptables` rules. +- **⚠️ Issues**: No Windows/macOS support, no persistence, no DNS leak protection. + +--- + +## 🚨 High-Risk Tech Debt +| Issue | Risk | Fix | +|-------|------|-----| +| **Race conditions in `useConnectionStore`** | Medium | Add `pending` flag to `connect`/`disconnect`. | +| **No `Mutex` timeout in IPC commands** | High | Use `tokio::time::timeout` for `Mutex` acquisition. | +| **Placeholder WireGuard keys** | Critical | Fail fast if `wg` tools are missing. | +| **No killswitch persistence** | High | Save state to disk and recover on startup. | +| **No input validation in IPC** | High | Validate `server_id` and other inputs. | +| **No error logging in frontend** | Medium | Add error boundaries and logging. | +| **No state validation on rehydration** | Medium | Validate persisted state (e.g., `user`, `tokens`). | +| **Latency measurement timeouts** | Medium | Add `tokio::time::timeout` for pings. | + +--- + +## ✅ Production Hardening Checklist +### **1. State Management** +- [ ] Add `pending` flag to `useConnectionStore` to prevent race conditions. +- [ ] Validate persisted state on rehydration (e.g., `user`, `tokens`). +- [ ] Replace `Map` with plain object for `latencyMap` persistence. +- [ ] Add error logging in Zustand actions. + +### **2. IPC Boundaries** +- [ ] Add `tokio::time::timeout` for `Mutex` acquisition in Tauri commands. +- [ ] Validate all IPC inputs (e.g., `server_id`, `email`). +- [ ] Replace mock data with real API calls (add `#[cfg(debug_assertions)]` for mocks). +- [ ] Add error boundaries in frontend for IPC errors. + +### **3. Configuration Management** +- [ ] Fail fast if `wg` tools are missing or keys cannot be generated. +- [ ] Make DNS/MTU configurable via `SettingsStore`. +- [ ] Add runtime config reloading (e.g., Tauri events). +- [ ] Validate `WireGuardConfig` with `schemars` or `validator`. + +### **4. Error Handling & Logging** +- [ ] Use `thiserror` or `anyhow` for structured errors. +- [ ] Add `RUST_LOG` env var support for log level filtering. +- [ ] Redact sensitive data in logs (e.g., tokens, IPs). +- [ ] Add error logging in frontend (e.g., Sentry). + +### **5. Killswitch & Network Safety** +- [ ] Implement platform-specific killswitch logic (Windows: WFP, macOS: `pf`). +- [ ] Persist killswitch state to disk and recover on startup. +- [ ] Add DNS leak protection (block port 53). +- [ ] Implement VPN interface detection for Windows/macOS. + +### **6. Build & CI/CD** +- [ ] Add `cargo clippy` and `cargo audit` to CI. +- [ ] Enforce `RUSTFLAGS="-D warnings"` in production builds. +- [ ] Add `tauri-bundler` for cross-platform packaging. +- [ ] Sign binaries for Windows/macOS. + +--- + +## 🛠️ Top 2 Fixes Implemented +1. **Race Conditions in `useConnectionStore`** + - Added `pending` flag to `connect`/`disconnect` to prevent overlapping calls. +2. **Placeholder WireGuard Keys** + - Fail fast if `wg` tools are missing or keys cannot be generated. + +--- + +## 📌 Recommendations +- **Short-Term**: Implement the **top 2 fixes** and address **high-risk issues**. +- **Long-Term**: Adopt **structured logging**, **error monitoring**, and **platform-specific killswitch** logic. +- **Testing**: Add **integration tests** for IPC commands and **fuzz testing** for config parsing. \ No newline at end of file diff --git a/src-tauri/src/config.rs b/src-tauri/src/config.rs index cb41604..202c27a 100644 --- a/src-tauri/src/config.rs +++ b/src-tauri/src/config.rs @@ -104,13 +104,9 @@ impl WireGuardConfig { /// Generate a new WireGuard keypair pub fn generate_keypair() -> Result<(String, String)> { // Try to use wg genkey/wg pubkey - match generate_keypair_native() { - Ok(keys) => Ok(keys), - Err(_) => { - // Fallback to generating keys with base64 - generate_keypair_fallback() - } - } + generate_keypair_native().map_err(|e| { + format!("Failed to generate WireGuard keypair: {}. Ensure `wg` tools are installed.", e) + }) } fn generate_keypair_native() -> Result<(String, String)> { @@ -176,18 +172,8 @@ pub struct Server { /// Generate a WireGuard configuration for a given server pub fn generate_wireguard_config(server_id: &str) -> Result { - // In a real implementation, this would fetch server details from the API - // and generate appropriate keys - // Generate client keypair - let (private_key, _public_key) = generate_keypair() - .unwrap_or_else(|_| { - // Use placeholder keys for demo - ( - "CLIENT_PRIVATE_KEY_PLACEHOLDER".to_string(), - "CLIENT_PUBLIC_KEY_PLACEHOLDER".to_string(), - ) - }); + let (private_key, _public_key) = generate_keypair()?; // Server public key (would come from API) let server_public_key = "SERVER_PUBLIC_KEY_PLACEHOLDER".to_string(); diff --git a/src/stores/index.ts b/src/stores/index.ts index eb7d69e..4a88148 100644 --- a/src/stores/index.ts +++ b/src/stores/index.ts @@ -140,6 +140,7 @@ interface ConnectionStoreState extends ConnectionState { updateStats: (bytesReceived: number, bytesSent: number) => void; setError: (error: string | undefined) => void; setIPInfo: (ipInfo: IPInfo | undefined) => void; + pending: boolean; } export const useConnectionStore = create()( @@ -151,12 +152,16 @@ export const useConnectionStore = create()( bytesSent: 0, error: undefined, ipInfo: undefined, + pending: false, connect: async (server: Server) => { - const { status } = get(); - if (status === "connecting" || status === "connected") { + const { status, pending } = get(); + if (status === "connecting" || status === "connected" || pending) { return; } + set((state) => { + state.pending = true; + }); set((state) => { state.status = "connecting"; @@ -173,22 +178,27 @@ export const useConnectionStore = create()( state.connectedAt = new Date(); state.bytesReceived = 0; state.bytesSent = 0; + state.pending = false; }); } catch (error) { set((state) => { state.status = "error"; state.error = error instanceof Error ? error.message : "Connection failed"; state.server = undefined; + state.pending = false; }); throw error; } }, disconnect: async () => { - const { status } = get(); - if (status === "disconnecting" || status === "disconnected") { + const { status, pending } = get(); + if (status === "disconnecting" || status === "disconnected" || pending) { return; } + set((state) => { + state.pending = true; + }); set((state) => { state.status = "disconnecting"; @@ -206,11 +216,13 @@ export const useConnectionStore = create()( state.bytesSent = 0; state.error = undefined; state.ipInfo = undefined; + state.pending = false; }); } catch (error) { set((state) => { state.status = "error"; state.error = error instanceof Error ? error.message : "Disconnection failed"; + state.pending = false; }); throw error; }