Files
ForgeLM/DIFFERENTIAL_REVIEW_REPORT.md

6.2 KiB

Differential Security & Code Review Report

1. Executive Summary

This review covers the last 10 commits focusing on the ForgeLM Tauri React UI, Rust inference engine, and ForgeAgent integrations. The analysis prioritized high-risk areas including inference performance degradation, prompt leakage/injection, UI state management, and authentication/security boundaries.

Overall Risk Level: MEDIUM The codebase introduces several defensive mechanisms against prompt leakage and UI state inconsistencies. However, it also introduces aggressive repetition penalties that may severely degrade code generation capabilities, and a prompt leakage regex that is overly broad and prone to false positives.


2. Detailed Findings by Risk Category

2.1 Inference Server & Performance Degradation (HIGH RISK)

Files Affected:

  • crates/inference-core/src/sampler.rs
  • crates/inference-core/src/session.rs
  • crates/inference-server/src/compat.rs
  • crates/llama-sys/src/lib.rs

Analysis:

  1. Aggressive Repetition Penalty: In crates/inference-server/src/compat.rs, the repeat_penalty has been increased from 1.05 to 1.3, and repeat_last_n set to 512 when no tools are provided.
    • Impact: A repetition penalty of 1.3 is extremely aggressive for LLMs. While it prevents loops, it will severely degrade the model's ability to generate code, JSON, or any structured text that naturally requires repeating keywords, brackets, or variable names.
    • Recommendation: Revert to 1.1 - 1.15 for general chat, or make the penalty configurable per-request.
  2. TPS Degradation Monitor is Passive: GenerationSession now includes a TPS degradation monitor (TpsDegradationConfig).
    • Impact: The monitor correctly identifies when TPS drops below 30.0 for 5 seconds, but it only emits a tracing::warn!. It does not abort the generation or notify the client, leaving the client hanging on a degraded session.
    • Recommendation: Expose the degradation state to the server router to allow early termination or client-side warnings via SSE events.

2.2 Prompt Injection & Transcript Leakage (MEDIUM RISK)

Files Affected:

  • tauri-app/src/lib/chat.ts
  • crates/inference-server/src/compat.rs

Analysis:

  1. Overly Broad Regex Truncation: To prevent the model from leaking transcript roles, the frontend introduced TRANSCRIPT_LEAKAGE_PATTERN = /(?:^|\n)\s*(?:System|User|Assistant|Human):/i;. The stripTranscriptLeakage function aggressively cuts off any text matching this regex.
    • Impact: This regex will trigger false positives. If a user asks "What is the role of a System Administrator:" or the model generates a markdown list like:
      Roles:
      - User: Performs actions
      - System: Manages resources
      
      The response will be abruptly truncated at "User:".
    • Recommendation: Rely strictly on exact backend stop_sequences (which was already implemented via extra_stop_sequences in compat.rs) rather than fuzzy frontend regex truncation, or significantly tighten the regex to expect specific formatting.
  2. Backend Stop Sequences: The backend correctly injects \nUser:, \nSystem:, etc., into extra_stop_sequences for the Generic chat template. This is a secure and standard approach.

2.3 UI State Corruption (LOW/MEDIUM RISK)

Files Affected:

  • tauri-app/src/features/runtime/useRuntimeManager.ts
  • tauri-app/src/components/ChatView.tsx

Analysis:

  1. Hidden Side-Effects in State Management: In useRuntimeManager.ts, an auto-scan was added inside commitSavedSnapshot via a useRef hook (scanModelsRef.current?.(modelsDir)).
    • Impact: While this fixes a stale UI issue, calling an async state-mutating function from within another state reconciliation flow (inside a .then block) can lead to race conditions. If the user rapidly toggles settings, the delayed scan could overwrite a newer draft state.
    • Recommendation: Decouple the model scanning from the snapshot commit, or use a dedicated useEffect that watches for runtimeSnapshot.config.models_dir changes.
  2. Massive ChatView Refactor: The redesign to Manus-style tokens (AgenticToggle, StreamingBubble, etc.) successfully decomposes a monolithic component and appropriately isolates states. The messageQueue state is correctly maintained.

2.4 Auth & Multi-Agent ForgeAgent Integration (MEDIUM RISK)

Files Affected:

  • tauri-app/src-tauri/src/lib.rs

Analysis:

  1. Missing Input Sanitization in Agent Prompts: The start_forgeagent_session_command embeds the raw prompt directly into the JSON body sent to the local inference server:
    { "role": "user", "content": &prompt }
    
    • Impact: While this is a local server, if the ForgeAgent system relies on the model outputting a strict JSON schema (which it parses via serde_json::from_str), a prompt injection could force the model to break out of the JSON structure. The code handles this gracefully by falling back to treating the raw text as a summary (ForgeAgentPlanCompletedPayload { summary: content }), preventing crashes.
    • Recommendation: Enforce JSON schema validation natively on the inference server using response_format: { type: "json_object" } if supported by the engine, rather than just appending "Limite a cadeia de raciocínio..." to the system prompt.
  2. Duplicate Error Handling Flow: In start_forgeagent_session_command, if the runtime is not ready, it emits an error event via app_handle.emit AND returns an Err("Runtime not ready"). This may cause the frontend to display duplicate toast notifications or error states.

3. Summary of Recommendations

  1. Revert Repetition Penalty: Lower repeat_penalty in compat.rs from 1.3 to 1.15 to restore coding and structured generation capabilities.
  2. Refine Leakage Prevention: Remove the broad TRANSCRIPT_LEAKAGE_PATTERN regex in chat.ts and rely entirely on the exact stop_sequences enforced by the backend.
  3. Handle TPS Degradation: Implement an active response to TPS degradation (e.g., aborting the stream with a specific error chunk) rather than just logging.
  4. Fix Race Conditions: Move the model auto-scanning logic into a reactive useEffect tied to the active models directory rather than an imperative ref call during snapshot commits.