6.2 KiB
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.rscrates/inference-core/src/session.rscrates/inference-server/src/compat.rscrates/llama-sys/src/lib.rs
Analysis:
- Aggressive Repetition Penalty: In
crates/inference-server/src/compat.rs, therepeat_penaltyhas been increased from1.05to1.3, andrepeat_last_nset to512when 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.15for general chat, or make the penalty configurable per-request.
- TPS Degradation Monitor is Passive:
GenerationSessionnow 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.
- Impact: The monitor correctly identifies when TPS drops below 30.0 for 5 seconds, but it only emits a
2.2 Prompt Injection & Transcript Leakage (MEDIUM RISK)
Files Affected:
tauri-app/src/lib/chat.tscrates/inference-server/src/compat.rs
Analysis:
- 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;. ThestripTranscriptLeakagefunction 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:
The response will be abruptly truncated at "User:".
Roles: - User: Performs actions - System: Manages resources - Recommendation: Rely strictly on exact backend
stop_sequences(which was already implemented viaextra_stop_sequencesincompat.rs) rather than fuzzy frontend regex truncation, or significantly tighten the regex to expect specific formatting.
- 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:
- Backend Stop Sequences: The backend correctly injects
\nUser:,\nSystem:, etc., intoextra_stop_sequencesfor theGenericchat template. This is a secure and standard approach.
2.3 UI State Corruption (LOW/MEDIUM RISK)
Files Affected:
tauri-app/src/features/runtime/useRuntimeManager.tstauri-app/src/components/ChatView.tsx
Analysis:
- Hidden Side-Effects in State Management: In
useRuntimeManager.ts, an auto-scan was added insidecommitSavedSnapshotvia auseRefhook (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
.thenblock) 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
useEffectthat watches forruntimeSnapshot.config.models_dirchanges.
- Impact: While this fixes a stale UI issue, calling an async state-mutating function from within another state reconciliation flow (inside a
- Massive ChatView Refactor: The redesign to Manus-style tokens (
AgenticToggle,StreamingBubble, etc.) successfully decomposes a monolithic component and appropriately isolates states. ThemessageQueuestate is correctly maintained.
2.4 Auth & Multi-Agent ForgeAgent Integration (MEDIUM RISK)
Files Affected:
tauri-app/src-tauri/src/lib.rs
Analysis:
- Missing Input Sanitization in Agent Prompts: The
start_forgeagent_session_commandembeds the rawpromptdirectly 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.
- Impact: While this is a local server, if the ForgeAgent system relies on the model outputting a strict JSON schema (which it parses via
- Duplicate Error Handling Flow: In
start_forgeagent_session_command, if the runtime is not ready, it emits an error event viaapp_handle.emitAND returns anErr("Runtime not ready"). This may cause the frontend to display duplicate toast notifications or error states.
3. Summary of Recommendations
- Revert Repetition Penalty: Lower
repeat_penaltyincompat.rsfrom1.3to1.15to restore coding and structured generation capabilities. - Refine Leakage Prevention: Remove the broad
TRANSCRIPT_LEAKAGE_PATTERNregex inchat.tsand rely entirely on the exactstop_sequencesenforced by the backend. - Handle TPS Degradation: Implement an active response to TPS degradation (e.g., aborting the stream with a specific error chunk) rather than just logging.
- Fix Race Conditions: Move the model auto-scanning logic into a reactive
useEffecttied to the active models directory rather than an imperative ref call during snapshot commits.