Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 95 additions & 0 deletions AI_REPORTS/architecture.md
Original file line number Diff line number Diff line change
@@ -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<Mutex<ConnectionManager>>` 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.
22 changes: 4 additions & 18 deletions src-tauri/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)> {
Expand Down Expand Up @@ -176,18 +172,8 @@ pub struct Server {

/// Generate a WireGuard configuration for a given server
pub fn generate_wireguard_config(server_id: &str) -> Result<WireGuardConfig> {
// 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();
Expand Down
20 changes: 16 additions & 4 deletions src/stores/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ConnectionStoreState>()(
Expand All @@ -151,12 +152,16 @@ export const useConnectionStore = create<ConnectionStoreState>()(
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";
Expand All @@ -173,22 +178,27 @@ export const useConnectionStore = create<ConnectionStoreState>()(
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";
Expand All @@ -206,11 +216,13 @@ export const useConnectionStore = create<ConnectionStoreState>()(
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;
}
Expand Down
Loading