Skip to content

COMPREHENSIVE_TODO_AND_BUGS.md

Latest commit

 

History

History
1267 lines (994 loc) · 32.6 KB

COMPREHENSIVE_TODO_AND_BUGS.md

File metadata and controls

1267 lines (994 loc) · 32.6 KB

Comprehensive TODO, Bug Report, and Code Review

Generated: 2026-02-02
Scope: Complete codebase review including Rust backend, TypeScript frontend, and prompt logic
Total Issues Identified: 50+ (Critical: 8, Major: 18, Minor: 25+)

🔴 CRITICAL ISSUES (Security & Data Loss)

C1. SQL Injection Risk in Encrypted DB

File: crates/hqe-core/src/encrypted_db.rs
Lines: 307-311, 277-280
Severity: CRITICAL - Security Vulnerability

// VULNERABLE CODE:
conn.execute(
    &format!("VACUUM INTO '{}'", escape_sql_string(&backup_path.to_string_lossy())),
    [],
)?;

Problem:

  • escape_sql_string only handles single quotes, but file paths can contain other SQL-injectable characters
  • VACUUM INTO with string concatenation is dangerous
  • No path validation before SQL execution

Impact:

  • Potential arbitrary file overwrite via SQL injection
  • Database corruption
  • Information disclosure

Fix:

// Use parameterized queries or proper path validation
let canonical_path = backup_path.canonicalize()
    .map_err(|e| EncryptedDbError::Io(e))?;
    
// Validate path doesn't contain suspicious patterns
if canonical_path.to_string_lossy().contains('"') 
    || canonical_path.to_string_lossy().contains('\0') {
    return Err(EncryptedDbError::Validation("Invalid path characters".into()));
}

C2. SQL Injection in Key Rotation

File: crates/hqe-core/src/encrypted_db.rs
Line: 277-280
Severity: CRITICAL

conn.execute(
    &format!("PRAGMA rekey = '{}'", escape_sql_string(&new_key)),
    [],
)?;

Problem: Same as C1 - key material is user-influenced and directly concatenated.

Fix: Use SQLCipher's parameterized key setting via pragma_update.

C3. Race Condition in Chat Session Creation

File: desktop/workbench/src/components/ConversationPanel.tsx
Lines: 91-105, 107-134
Severity: CRITICAL - Data Corruption

const createNewSession = useCallback(async () => {
  // ... creates session
  setCurrentSession(session)  // State update 1
  setMessages([])             // State update 2 - NOT ATOMIC
}, [])

Problem:

  • Multiple rapid session creations can lead to race conditions
  • Zustand state updates are not batched atomically
  • User could end up with messages in wrong session

Impact:

  • Cross-session message contamination
  • Data loss
  • Inconsistent UI state

Fix:

// Use atomic update
setChatState({
  currentSession: session,
  messages: [],
})

C4. Missing Input Sanitization in Prompt Templates

File: desktop/workbench/src-tauri/src/prompts.rs
Line: 316-330
Severity: CRITICAL - Prompt Injection

fn substitute_template(template: &str, args: &serde_json::Value) -> String {
    let mut result = template.to_string();
    if let Some(obj) = args.as_object() {
        for (k, v) in obj {
            let key = format!("{{{{{}}}}}", k); // {{key}}
            let val = v.as_str().map(sanitize_for_prompt).unwrap_or_else(...);
            result = result.replace(&key, &val);  // NO VALIDATION OF KEY!
        }
    }
    result
}

Problem:

  • Key names are not validated - could contain }} to break out of template
  • No check for recursive template injection
  • Keys like }}{{ malicious_key }} could inject content

Impact:

  • Prompt injection attacks
  • System prompt bypass
  • Information disclosure

Fix:

// Validate key names
let valid_key_regex = regex::Regex::new(r"^[a-zA-Z_][a-zA-Z0-9_]*$").unwrap();
for (k, v) in obj {
    if !valid_key_regex.is_match(k) {
        return Err(format!("Invalid template key: {}", k));
    }
    // ... rest of substitution
}

C5. Mutex Poisoning Panic Risk

File: crates/hqe-core/src/encrypted_db.rs
Multiple locations
Severity: CRITICAL - Denial of Service

let conn = self.conn.lock().map_err(|_| {
    EncryptedDbError::Encryption("Mutex poisoned".to_string())
})?;

Problem:

  • While this converts poison to error, the error handling may not be sufficient
  • If a thread panics while holding the lock, subsequent operations fail
  • No recovery mechanism implemented

Impact:

  • Complete database unavailability until app restart
  • Data loss for in-flight operations

Fix: Implement proper mutex recovery or use parking_lot mutex which doesn't poison.

C6. Insecure Temporary File Handling

File: crates/hqe-core/src/encrypted_db.rs
Lines: Tests use tempfile::tempdir()
Severity: CRITICAL (Test Environment Only)

Problem: Tests create real encrypted databases in system temp directories. If tests panic, temp files may not be cleaned up, leaving decrypted database fragments.

Fix: Ensure all test databases use #[cfg(feature = "sqlcipher-tests")] and are properly cleaned up via RAII.

C7. XSS Vulnerability in ConversationPanel

File: desktop/workbench/src/components/ConversationPanel.tsx
Lines: 330-354
Severity: CRITICAL - Client-Side Code Execution

<ReactMarkdown
  remarkPlugins={[remarkGfm]}
  components={{
    code({ inline, className, children, ...props }) {
      // ... renders user content without sanitization
    }
  }}
>
  {message.content}  // DIRECT RENDERING OF LLM OUTPUT
</ReactMarkdown>

Problem:

  • LLM output is rendered directly as markdown without sanitization
  • Malicious LLM responses could contain XSS payloads: [click me](javascript:alert('xss'))
  • Raw HTML in markdown is passed through

Impact:

  • Arbitrary JavaScript execution
  • Session hijacking
  • Data exfiltration

Fix:

import DOMPurify from 'dompurify';

// Sanitize content before rendering
const sanitizedContent = DOMPurify.sanitize(message.content, {
  ALLOWED_TAGS: ['p', 'br', 'strong', 'em', 'code', 'pre'],
  ALLOWED_ATTR: []
});

C8. Race Condition in Message Sending

File: desktop/workbench/src/components/ConversationPanel.tsx
Lines: 154-182
Severity: CRITICAL

const handleSend = useCallback(async () => {
  // ...
  const response = await invoke<{ message: ChatMessage }>('send_chat_message', {
    session_id: currentSession.id,
    content,
    parent_id: messages[messages.length - 1]?.id,  // STALE REFERENCE!
  })
  addMessage(response.message)
}, [messages, ...])  // messages is captured in closure

Problem:

  • messages array is captured in closure but may be stale
  • Rapid successive sends could attach to wrong parent
  • State inconsistency between frontend and backend

Fix: Use functional state updates:

const parentId = await new Promise<string | undefined>(resolve => {
  setMessages(currentMessages => {
    resolve(currentMessages[currentMessages.length - 1]?.id);
    return currentMessages;
  });
});

🟠 MAJOR ISSUES (Functionality & Reliability)

M1. Inefficient Database Connection Management

File: desktop/workbench/src-tauri/src/chat.rs
All command functions
Severity: MAJOR - Performance

Problem: Every Tauri command creates a new EncryptedDb::init() connection:

#[command]
pub async fn create_chat_session(...) -> Result<...> {
    let db = EncryptedDb::init().map_err(|e| e.to_string())?;  // NEW CONNECTION EVERY TIME!
    // ...
}

Impact:

  • Connection overhead on every operation
  • Keychain lookup on every call
  • No connection pooling

Fix: Use Tauri state to hold a singleton database instance:

pub struct AppState {
    pub db: Arc<Mutex<EncryptedDb>>,
}

#[command]
pub async fn create_chat_session(
    state: tauri::State<'_, AppState>,
    ...
) -> Result<...> {
    let db = state.db.lock().await;
    // ... use existing connection
}

M2. Inconsistent Prompt Category Duplication

File: crates/hqe-core/src/prompt_runner.rs vs crates/hqe-mcp/src/registry_v2.rs
Severity: MAJOR - Maintainability

Problem: Two separate PromptCategory enums exist with different variants:

// prompt_runner.rs - 7 variants
pub enum PromptCategory {
    Security, Quality, Refactor, Explain, Test, Document, Custom,
}

// registry_v2.rs - 12 variants  
pub enum PromptCategory {
    Uncategorized, Security, Quality, Refactor, Explain, Test, 
    Document, Architecture, Performance, Dependencies, Custom, Agent,
}

Impact:

  • Category mapping bugs
  • Inconsistent UI behavior
  • Data serialization issues

Fix: Consolidate to a single enum in hqe-protocol crate.

M3. Unbounded Message Loading

File: crates/hqe-core/src/encrypted_db.rs
Lines: 592-628
Severity: MAJOR - Performance/DoS

fn get_messages(&self, session_id: &str) -> Result<Vec<ChatMessage>> {
    // ... loads ALL messages without pagination
    let rows: Vec<ChatMessage> = stmt
        .query_map([session_id], |row| { ... })?
        .filter_map(|r| r.ok())
        .collect();  // COULD BE MILLIONS OF ROWS
    Ok(rows)
}

Impact:

  • Memory exhaustion on long chat sessions
  • UI freezing
  • Potential OOM crash

Fix: Implement pagination:

fn get_messages(&self, session_id: &str, limit: usize, offset: usize) -> Result<Vec<ChatMessage>> {
    // Add LIMIT/OFFSET to SQL query
}

M4. Missing Transaction Support

File: crates/hqe-core/src/encrypted_db.rs
Severity: MAJOR - Data Integrity

Problem: No transaction support for multi-operation updates:

fn add_message(&self, message: &ChatMessage) -> Result<()> {
    conn.execute("INSERT INTO chat_messages ...")?;  // Op 1
    // If crash happens here, session timestamp is not updated
    conn.execute("UPDATE chat_sessions SET updated_at = ...")?;  // Op 2
    Ok(())
}

Impact:

  • Inconsistent state on crash
  • Orphaned messages
  • Incorrect session ordering

Fix: Wrap in transactions:

fn add_message(&self, message: &ChatMessage) -> Result<()> {
    let tx = conn.transaction()?;
    tx.execute("INSERT INTO chat_messages ...")?;
    tx.execute("UPDATE chat_sessions ...")?;
    tx.commit()?;
    Ok(())
}

M5. Inefficient Prompt Discovery

File: crates/hqe-mcp/src/loader.rs
Lines: 102-125
Severity: MAJOR - Performance

pub fn load(&self) -> Result<Vec<LoadedPromptTool>, LoaderError> {
    for entry in WalkDir::new(&self.root_path)
        .follow_links(true)  // DANGEROUS - can traverse entire filesystem!
        .into_iter()
        // ...
}

Impact:

  • Symlink attacks can traverse outside intended directory
  • Infinite loops with circular symlinks
  • Full filesystem scans possible

Fix:

pub fn load(&self) -> Result<Vec<LoadedPromptTool>, LoaderError> {
    for entry in WalkDir::new(&self.root_path)
        .follow_links(false)  // NEVER follow symlinks
        .max_depth(5)         // Limit recursion
        // ...
}

M6. Error Information Leakage

File: desktop/workbench/src-tauri/src/chat.rs
All command functions
Severity: MAJOR - Information Disclosure

pub async fn create_chat_session(...) -> Result<ChatSessionDto, String> {
    let db = EncryptedDb::init().map_err(|e| e.to_string())?;  // LEAKS INTERNAL ERROR
}

Problem: Internal error details exposed to frontend:

  • Database file paths
  • SQL errors revealing schema
  • Keychain service names

Fix: Log detailed errors internally, return generic messages:

let db = EncryptedDb::init().map_err(|e| {
    error!(error = %e, "Failed to initialize database");  // Log full error
    "Failed to initialize chat database".to_string()      // Generic user message
})?;

M7. No Rate Limiting on Chat

File: desktop/workbench/src-tauri/src/chat.rs
Severity: MAJOR - Resource Exhaustion

Problem: send_chat_message has no rate limiting. Users can spam messages causing:

  • Database bloat
  • LLM API quota exhaustion
  • UI freezing from rapid updates

Fix: Implement rate limiting:

use std::sync::atomic::{AtomicU64, Ordering};

static LAST_MESSAGE_TIME: AtomicU64 = AtomicU64::new(0);

pub async fn send_chat_message(...) -> Result<...> {
    let now = std::time::SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_secs();
    let last = LAST_MESSAGE_TIME.swap(now, Ordering::SeqCst);
    if now - last < 1 {  // 1 second between messages
        return Err("Please wait before sending another message".into());
    }
    // ...
}

M8. Unused showIncompatible State

File: desktop/workbench/src/screens/ThinktankScreen.tsx
Line: 131
Severity: MAJOR - Incomplete Feature

const [showIncompatible, setShowIncompatible] = useState(false)  // NEVER USED!

The UI shows a checkbox for "Show incompatible prompts" but the filtering logic doesn't implement this feature.

Fix: Either implement the filter or remove the UI element.

M9. Unsafe Regex Compilation

File: crates/hqe-core/src/prompt_runner.rs
Line: 323-328
Severity: MAJOR - DoS

if let Some(pattern) = &spec.validation {
    let regex = regex::Regex::new(pattern)  // COMPILES USER-PROVIDED REGEX!
        .map_err(|e| PromptRunnerError::InvalidInput { ... })?;
}

Problem: User-provided regex patterns can be malicious:

  • Regex bombs: (a+)+ on input aaaaaaaaaaaaaaaaaaaaaaaaaaaa!
  • Stack overflow on deeply nested patterns
  • Exponential backtracking

Fix: Use regex::RegexBuilder with limits:

let regex = regex::RegexBuilder::new(pattern)
    .size_limit(1024 * 1024)  // 1MB limit
    .dfa_size_limit(1024 * 1024)
    .build()
    .map_err(|e| ...)?;

M10. Weak Jailbreak Detection

File: crates/hqe-core/src/system_prompt.rs
Lines: 186-208
Severity: MAJOR - Security Bypass

Problem: Pattern matching is too simplistic:

let patterns = [
    "ignore previous",      // Matches "ignore previous" but not "ıgnore previοus" (homoglyphs)
    "reveal your system prompt",  // No semantic analysis
    // ...
];

Bypass Examples:

  • "ıgnore previοus" (using Unicode homoglyphs)
  • "ignore\nprevious" (newline bypass)
  • "i g n o r e previous" (spacing)
  • Base64 encoded attacks decoded by model

Fix: Implement multi-layer defense:

  1. Unicode normalization (NFKD)
  2. Semantic analysis using embeddings
  3. Context-aware detection
  4. Rate limiting on suspicious patterns

M11. Context Block Truncation Bug

File: crates/hqe-core/src/prompt_runner.rs
Lines: 407-421
Severity: MAJOR - Data Loss

if total_size > max_size {
    blocks.push(format!(
        "--- BEGIN UNTRUSTED CONTEXT ---\nSource: {}\nType: {:?}\n\n[Content truncated due to size limit]\n\n--- END UNTRUSTED CONTEXT ---",
        ctx.source, ctx.content_type
    ));
    break;  // DROPS ALL REMAINING CONTEXT!
}

Problem: When size limit is exceeded, ALL remaining context is dropped, not just truncated.

Fix: Implement proper truncation that keeps as much context as possible.

M12. Insecure Backup Path Handling

File: crates/hqe-core/src/encrypted_db.rs
Line: 297-316
Severity: MAJOR - File Overwrite

pub fn export_backup(&self, backup_path: &PathBuf) -> Result<()> {
    // No validation that backup_path is safe!
    conn.execute(&format!("VACUUM INTO '{}'", ...), [])?;
}

Problem: Can overwrite arbitrary files:

  • ../../../etc/passwd (if permissions allow)
  • Existing database files
  • System files

Fix: Validate path is within allowed directory:

let canonical = backup_path.canonicalize()?;
let allowed = std::env::current_dir()?.join("backups");
if !canonical.starts_with(&allowed) {
    return Err(EncryptedDbError::Validation("Backup path outside allowed directory".into()));
}

M13. No Input Length Limits

File: Multiple files
Severity: MAJOR - DoS

Problem: No maximum length validation on:

  • Chat messages (can be gigabytes)
  • Prompt template inputs
  • User search queries

Impact:

  • Memory exhaustion
  • Database bloat
  • UI freezing

Fix: Add length limits throughout:

const MAX_MESSAGE_LENGTH: usize = 100_000;  // 100KB
const MAX_INPUT_LENGTH: usize = 10_000;     // 10KB

M14. Frontend Error Handling Deficiencies

File: desktop/workbench/src/components/ConversationPanel.tsx
Multiple locations
Severity: MAJOR - UX

try {
  // ... operation
} catch (err) {
  console.error('Failed to load session:', err)
  toast.error('Failed to load chat session')  // GENERIC MESSAGE!
}

Problem:

  • All errors show generic "Failed to X" messages
  • Users don't know if it's a network error, auth error, or bug
  • No retry mechanisms

Fix: Implement structured error handling:

try {
  // ...
} catch (err) {
  if (err.message?.includes('network')) {
    toast.error('Network error. Please check your connection.');
  } else if (err.message?.includes('not found')) {
    toast.error('Session not found. It may have been deleted.');
  } else {
    toast.error('An unexpected error occurred. Please try again.');
    reportError(err);  // Send to error tracking
  }
}

M15. Zustand Store Not Persisting Chat

File: desktop/workbench/src/store.ts
Lines: 84-102
Severity: MAJOR - Data Loss Risk

export const useChatStore = create<ChatState>()(
  persist(
    (set) => ({ ... }),
    { name: 'hqe-chat-storage' }  // ONLY PERSISTS STATE, NOT ACTUAL MESSAGES!
  )
)

Problem: Zustand persist only saves the React state, not the actual database content. If user clears browser storage, they lose all chat history even though it's in the encrypted DB.

Fix: Don't persist chat state - always load from encrypted DB on mount.

M16. Memory Leak in useEffect Dependencies

File: desktop/workbench/src/screens/ThinktankScreen.tsx
Lines: 151-178, 233
Severity: MAJOR - Memory Leak

useEffect(() => {
    let filtered = prompts
    // ... complex filtering logic runs on every render cycle dependency change
    setFilteredPrompts(filtered)
}, [prompts, searchQuery, selectedCategory, showAgentPrompts, isAgentPrompt])

Problem:

  • isAgentPrompt is a function that changes reference on every render
  • Causes effect to run continuously
  • Memory accumulation from closure captures

Fix: Wrap isAgentPrompt in useCallback with empty deps (it's pure).

M17. SQLCipher Feature Gate Not Documented

File: crates/hqe-core/Cargo.toml
Severity: MAJOR - Developer Experience

Problem: Tests are feature-gated behind sqlcipher-tests but:

  • No documentation explains this
  • CI may not be running these tests
  • Developers get confusing "0 tests" output

Fix:

  1. Add to README.md
  2. Add #[ignore = "requires sqlcipher feature"] with clear message
  3. Add CI job that runs with feature enabled

M18. Deadlock Risk in Tauri Commands

File: desktop/workbench/src-tauri/src/chat.rs
Lines: 211-246
Severity: MAJOR - Deadlock

pub async fn send_chat_message(...) -> Result<...> {
    let db = EncryptedDb::init()?;  // Await holding no lock
    // ... but what if this is called rapidly?
}

Problem: While current implementation doesn't show deadlock, adding connection pooling later could cause:

  • Connection pool exhaustion
  • Circular wait conditions
  • Async runtime blocking

Fix: Use bounded channels for database operations with timeouts.

🟡 MINOR ISSUES (Code Quality & Improvements)

m1. Unused Import Warnings

Files: Multiple
Severity: MINOR

Examples:

  • desktop/workbench/src-tauri/src/chat.rs:11 - error imported but not used
  • desktop/workbench/src-tauri/src/chat.rs:283 - AuthScheme imported but not used
  • Various frontend files with unused React imports

Fix: Run cargo fix and clean up TypeScript imports.

m2. Hardcoded Values Throughout

Files: Multiple
Severity: MINOR - Maintainability

Examples:

// prompt_runner.rs:229
max_context_bytes: 100_000,  // Why 100KB? Not configurable.

// encrypted_db.rs:79  
kdf_iterations: 256000,  // Should be configurable

// ConversationPanel.tsx:54
max-w-[85%]  // Magic number

Fix: Extract to constants/configuration:

pub const DEFAULT_MAX_CONTEXT_BYTES: usize = 100_000;
pub const MIN_KDF_ITERATIONS: i32 = 100_000;

m3. Missing Documentation on Public APIs

Files: Multiple
Severity: MINOR - Documentation

Many public functions lack rustdoc:

  • EncryptedDb::rotate_key
  • PromptRegistry::sorted
  • ProviderSpec::format_api_key

Fix: Add #![warn(missing_docs)] and fix all warnings.

m4. Inconsistent Error Types

Files: Multiple
Severity: MINOR - API Consistency

Some functions return Result<T, String>, others Result<T, CustomError>:

// chat.rs
pub async fn create_chat_session(...) -> Result<ChatSessionDto, String>

// encrypted_db.rs
pub fn init() -> Result<Self>  // Uses custom EncryptedDbError

Fix: Standardize on custom error types throughout.

m5. Redundant Cloning

Files: Multiple
Severity: MINOR - Performance

// registry_v2.rs:88
input_schema: t.definition.input_schema.clone(),  // Already owned!

Fix: Review clones and remove unnecessary ones.

m6. Test Naming Inconsistency

Files: Multiple test files
Severity: MINOR - Conventions

Some tests use test_ prefix, others don't:

#[test]
fn test_category_detection() { }  // With prefix

#[test]
fn category_detection() { }  // Without prefix

Fix: Standardize on test_ prefix for all tests.

m7. Magic Strings for Role Types

File: desktop/workbench/src-tauri/src/chat.rs
Lines: 160-166, 134-139
Severity: MINOR

let role_enum = match role.as_str() {
    "system" => MessageRole::System,
    "user" => MessageRole::User,
    // ... magic strings!
};

Fix: Use constants:

pub const ROLE_SYSTEM: &str = "system";
pub const ROLE_USER: &str = "user";
// ...

m8. Missing Index on Foreign Keys

File: crates/hqe-core/src/encrypted_db.rs
Lines: 242-254
Severity: MINOR - Performance

// Current indexes
CREATE INDEX idx_messages_session ON chat_messages(session_id)
CREATE INDEX idx_messages_timestamp ON chat_messages(timestamp)
CREATE INDEX idx_sessions_repo ON chat_sessions(repo_path)

// Missing:
CREATE INDEX idx_messages_parent ON chat_messages(parent_id)  -- For tree traversal
CREATE INDEX idx_feedback_message ON feedback(message_id)     -- For message feedback lookup

m9. Inefficient String Building

File: crates/hqe-mcp/src/registry_v2.rs
Lines: 415-438
Severity: MINOR - Performance

fn build_explanation(&self, description: &str, inputs: &[InputSpec]) -> String {
    let mut explanation = format!("## Purpose\n\n{}", description);
    // Repeated push_str - inefficient for large strings
    explanation.push_str("\n\n## Inputs\n\n");
    // ...
}

Fix: Use String::with_capacity or join:

let parts: Vec<String> = vec![
    format!("## Purpose\n\n{}", description),
    format!("\n\n## Inputs\n\n{}"),
    // ...
];
parts.join("")

m10. No Timeout on Tauri Invokes

File: desktop/workbench/src/components/ConversationPanel.tsx
Multiple locations
Severity: MINOR - UX

const session = await invoke<ChatSession>('create_chat_session', {
  // ... no timeout!
})

Problem: If backend hangs, UI is stuck indefinitely.

Fix: Add timeout wrapper:

const invokeWithTimeout = <T,>(cmd: string, args: any, timeoutMs: number): Promise<T> => {
  return Promise.race([
    invoke<T>(cmd, args),
    new Promise<never>((_, reject) => 
      setTimeout(() => reject(new Error('Timeout')), timeoutMs)
    )
  ]);
};

m11. Date Parsing Ignores Errors

File: crates/hqe-core/src/encrypted_db.rs
Lines: 489, 522, etc.
Severity: MINOR - Silent Failures

created_at: parse_datetime(row.get(5)?).unwrap_or_else(chrono::Utc::now),

Problem: Parsing failures silently use current time, potentially corrupting data ordering.

Fix: Log warning at minimum:

created_at: parse_datetime(row.get(5)?).unwrap_or_else(|| {
    warn!("Failed to parse datetime, using current time");
    chrono::Utc::now()
}),

m12. Unclear Session ID Generation

File: desktop/workbench/src-tauri/src/chat.rs
Line: 51
Severity: MINOR

id: Uuid::new_v4().to_string(),  // Random UUID

Problem: Non-sortable IDs make session listing order unpredictable.

Fix: Use ULID (lexicographically sortable):

id: ulid::Ulid::new().to_string(),  // Sortable, contains timestamp

m13. No Input Validation on Context Size

File: crates/hqe-core/src/prompt_runner.rs
Lines: 276-295
Severity: MINOR

pub fn build_prompt(&self, request: &PromptExecutionRequest) -> Result<String, PromptRunnerError> {
    // Validates inputs but not total prompt size!
    self.validate_inputs(request)?;
    // ... could generate multi-GB prompt
}

Fix: Add total size validation after building.

m14. Emoji in Category Icons

File: crates/hqe-mcp/src/registry_v2.rs
Lines: 83-97
Severity: MINOR - Compatibility

pub fn icon(&self) -> &'static str {
    match self {
        PromptCategory::Security => "🔒",  // May not render on all systems
        // ...
    }
}

Problem: Emoji rendering depends on system fonts. May show as □ on some systems.

Fix: Provide SVG icon alternatives or use icon font.

m15. React Key Warning Risk

File: desktop/workbench/src/components/ConversationPanel.tsx
Line: 211-215
Severity: MINOR - React Performance

{displayMessages.map((message, idx) => (
  <MessageBubble
    key={message.id || idx}  // Falls back to index - bad for reordering!
    message={message}
  />
))}

Problem: Using index as fallback key causes React to reuse components incorrectly if order changes.

Fix: Ensure all messages have stable IDs before rendering.

m16. No Debouncing on Search Input

File: desktop/workbench/src/screens/ThinktankScreen.tsx
Line: 247-252
Severity: MINOR - Performance

<input
  type="text"
  placeholder="Search prompts..."
  value={searchQuery}
  onChange={(e) => setSearchQuery(e.target.value)}  // Triggers effect on every keystroke!
  className="input text-sm"
/>

Problem: Filtering runs on every keystroke, blocking UI for large prompt lists.

Fix: Add debounce:

const [debouncedSearch] = useDebounce(searchQuery, 300);
useEffect(() => {
  // Filter using debouncedSearch
}, [debouncedSearch, ...]);

m17. Console Error Logging in Production

File: desktop/workbench/src/components/ConversationPanel.tsx
Multiple locations
Severity: MINOR

catch (err) {
  console.error('Failed to load session:', err)  // Visible in production!
  toast.error('Failed to load chat session')
}

Problem: Error details leak to browser console in production builds.

Fix: Use proper error reporting service:

catch (err) {
  if (process.env.NODE_ENV === 'development') {
    console.error('Failed to load session:', err);
  }
  errorReporter.capture(err);  // Send to Sentry/etc in production
  toast.error('Failed to load chat session');
}

m18. Unsafe as Casts in TypeScript

File: desktop/workbench/src/screens/ThinktankScreen.tsx
Line: 155
Severity: MINOR - Type Safety

const state = location.state as { promptName?: string; args?: Record<string, unknown> } | null

Problem: as cast bypasses type checking. Runtime value may not match type.

Fix: Use type guards:

function isPromptState(obj: unknown): obj is { promptName: string; args?: Record<string, unknown> } {
  return typeof obj === 'object' && obj !== null && 'promptName' in obj;
}

m19. Missing Accessibility Attributes

Files: All frontend components
Severity: MINOR - Accessibility

Examples:

  • Buttons without aria-label
  • Icons without aria-hidden
  • No focus management
  • Color contrast issues with Dracula theme

Fix: Run axe-core and fix all violations.

m20. No Loading State for Initial Load

File: desktop/workbench/src/screens/ThinktankScreen.tsx
Lines: 267-283
Severity: MINOR - UX

When prompts.length === 0, it shows "No prompts found" immediately before loading completes.

Fix: Separate loading state from empty state:

{isLoading ? (
  <LoadingSkeleton />
) : prompts.length === 0 ? (
  <EmptyState />
) : (...)

📋 ARCHITECTURAL CONCERNS

A1. Tight Coupling Between Registry and Loader

File: crates/hqe-mcp/src/registry_v2.rs
Severity: Architectural

PromptRegistry owns PromptLoader and calls .load() during initialization. This makes:

  • Testing difficult (requires real filesystem)
  • No separation between loading and caching
  • Can't use registry with different data sources

Recommendation: Implement trait-based abstraction:

trait PromptSource {
    fn load(&self) -> Result<Vec<LoadedPromptTool>, Error>;
}

struct FileSystemSource { ... }
struct MockSource { ... }

A2. Missing Caching Layer for Prompts

Files: Multiple
Severity: Architectural - Performance

Every call to get_available_prompts re-reads all files from disk:

pub async fn get_available_prompts(app: AppHandle) -> Result<Vec<PromptToolInfo>, String> {
    let loader = hqe_mcp::PromptLoader::new(&prompts_dir);
    let loaded_tools = loader.load().map_err(|e| e.to_string())?;  // DISK READ EVERY TIME!
    // ...
}

Recommendation: Implement file watcher-based caching:

pub struct PromptCache {
    prompts: Arc<RwLock<Vec<EnrichedPrompt>>>,
    last_modified: Arc<RwLock<HashMap<PathBuf, SystemTime>>>,
}

A3. Synchronous File Operations in Async Context

Files: Multiple Tauri commands
Severity: Architectural - Performance

pub async fn get_available_prompts(app: AppHandle) -> Result<...> {
    // This blocks the async runtime!
    let loaded_tools = loader.load().map_err(|e| e.to_string())?;
}

Recommendation: Use tokio::task::spawn_blocking:

let loaded_tools = tokio::task::spawn_blocking(move || {
    loader.load()
}).await.map_err(|e| e.to_string())??;

A4. No Event System for Chat Updates

File: desktop/workbench/src/components/ConversationPanel.tsx
Severity: Architectural - Real-time

Currently, chat updates require polling or manual refresh. No real-time synchronization.

Recommendation: Implement Tauri events:

// Backend emits when new message added
app.emit("chat:message_received", MessagePayload { ... });

// Frontend listens
listen("chat:message_received", (event) => { ... });

🎯 RECOMMENDED PRIORITY ORDER

Phase 1: Security (Immediate)

  1. C7 - XSS Vulnerability
  2. C1/C2 - SQL Injection
  3. C4 - Prompt Injection
  4. C3/C8 - Race Conditions

Phase 2: Stability (This Week)

  1. M1 - Database Connection Pooling
  2. M4 - Transaction Support
  3. M10 - Jailbreak Detection Improvements
  4. M14 - Error Handling

Phase 3: Performance (Next Sprint)

  1. M3 - Pagination
  2. M5 - Prompt Discovery Security
  3. M16 - Memory Leaks
  4. A2 - Caching

Phase 4: Polish (Ongoing)

  1. All Minor issues (m1-m20)
  2. Documentation improvements
  3. Test coverage expansion

📊 TEST COVERAGE GAPS

Module Current Coverage Gap
encrypted_db.rs ~60% SQLCipher tests feature-gated, backup/restore untested
prompt_runner.rs ~70% Provider integration tests missing
system_prompt.rs ~90% Good coverage
registry_v2.rs ~50% Category edge cases, error paths
loader.rs ~40% Malicious input testing incomplete
Frontend ~20% E2E tests missing, mostly smoke tests

Recommendations:

  1. Add property-based testing (proptest) for input validation
  2. Add fuzzing for prompt template parsing
  3. Add E2E tests with Playwright/Cypress
  4. Add performance benchmarks

📝 CONCLUSION

This codebase demonstrates solid architectural decisions but has several critical security vulnerabilities that need immediate attention:

  1. Security vulnerabilities (C1-C8) pose immediate risks and should be fixed before any production deployment
  2. Race conditions in the frontend could lead to data corruption
  3. Performance issues with database connections will scale poorly
  4. Missing features like pagination and caching will become critical as usage grows

The code quality is generally high with good error handling patterns in Rust, but the frontend needs more robust error handling and input validation.

Estimated effort to address all issues:

  • Critical: 3-4 days
  • Major: 1-2 weeks
  • Minor: 1 week
  • Architectural: 2-3 weeks

Total: 4-6 weeks for comprehensive hardening