security: unify the LLM pre-flight guard across all paths (agent, comparison, HTTP + chat) - #19
Merged
Conversation
…nd HTTP paths Prompt-injection scanning, sensitive-data + cloud-upload consent, budget and sensitivity routing were enforced only on the chat IPC path (ipc-handlers.ts). AgentExecutor.execute, ComparisonService.runComparison and the HTTP/WS pipeline (ServiceLayer.sendMessageStreaming) call the LLM directly and reached cloud providers with none of these guards — content that would be blocked or consent-gated in chat flowed straight to a cloud model. - New src/main/security/request-guard.ts: pure, dependency-light orchestration (preflightGuard + postflightGuard) with an injectable GuardDeps, so it is unit-testable without the Electron/model module graph. request-guard-deps.ts wires the real singletons (input sanitizer, sensitive-data detector, upload permission, budget manager, data classifier, output guardrails). - Wire preflightGuard into ServiceLayer.sendMessageStreaming, AgentExecutor.execute and ComparisonService.runSingleModel. Non-interactive callers fail CLOSED: a block (injection / sensitive-denied / consent-required / budget / routing) refuses the cloud call. Chat/agent honour a budget/routing provider switch; comparison keeps its chosen model and blocks only on a hard deny. Tests: request-guard.test.ts (red before the module existed) covers each guard. Existing agent/comparison/context tests get a pass-through GuardDeps mock so they test service logic, not the guard. Full suite: 1302 passed, tsc clean. Note: the chat IPC path keeps its existing inline guard (already protected). Follow-up: collapse that copy onto request-guard too (single source), and lift the circuit-breaker + canary-token checks into the shared guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…single source) The chat SEND_MESSAGE handler carried its own ~180-line copy of the injection / sensitive-data-consent / budget / routing chain — the very duplication that let the agent/comparison/HTTP paths drift out of guard coverage. This removes the copy so there is one implementation. - request-guard.ts is split into guardInput (injection + sensitive-data/upload consent) and guardDispatch (budget + routing); preflightGuard now composes them, so the non-interactive callers are unchanged. The chat path interleaves command handling between the two phases, so it calls guardInput before commands and guardDispatch after — same logic, same order, no behaviour change. - ipc-handlers.ts drops the inline chain (and its now-unused imports/consts) and calls the shared phases. The exact IPC surface is preserved: message:error / message:permission-required / message:safety-warning events and the injectionBlocked / sensitiveDataBlocked / pendingConsent return payloads are byte-identical; request-guard-deps forwards the full UploadPermissionResponse plus the built request so the consent dialog gets exactly what it did before. - Tests: guardInput/guardDispatch get direct coverage (consent surfacing, routing sensitivity/reasons, budget fallback/block). Full suite: 1310 passed, tsc clean. Follow-up unchanged: lift the circuit-breaker + canary-token checks into the shared guard too (still chat-only). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Baldri
changed the title
…essage InputSanitizer.sanitize() computes a `sanitized` string (invisible Unicode removed, Cyrillic/Greek homoglyphs normalised to Latin) but callers used it only for the safe/riskScore decision and sent the RAW message — so obfuscated content reached the model. - request-guard: SanitizeResult carries `sanitized`; guardInput returns it as sanitizedText on the ok outcome, and preflightGuard surfaces it, so every path can apply the cleaned text. - Chat path (ipc-handlers): the message that reaches the LLM is now derived from the sanitised text (`inputGuard.sanitizedText ?? userMessage`) — the exact spot the finding flagged. Test: guardInput surfaces the cleaned last message. Full suite: 1311 passed, tsc clean. Follow-up: the agent/comparison/HTTP paths now have sanitizedText available from preflightGuard too — apply it at their send sites as well. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Was
Ein einziger geteilter LLM-Guard (
request-guard.ts) auf allen Pfaden: Agent, Comparison, HTTP/WS und Chat. Prompt-Injection-Scan, PII-Scan + Cloud-Upload-Consent, Budget, Sensitivity-Routing.Warum
Die Schutzkette lief bisher nur inline auf dem Chat-IPC-Pfad — genau die Duplikation, die Agent/Comparison/HTTP aus der Deckung driften liess (Audit: rls-gap Medium). Jetzt gibt es eine Implementierung.
Änderungen
src/main/security/request-guard.ts(rein, dependency-arm, unit-testbar ohne die Electron-Modulkette). Zwei Phasen —guardInput(Injection + PII/Consent) undguardDispatch(Budget + Routing) — pluspreflightGuardals Komposition.request-guard-deps.tsverdrahtet die echten Singletons.ServiceLayer.sendMessageStreaming(HTTP/WS),AgentExecutor.execute,ComparisonService.runSingleModel. Ein Block verweigert den Cloud-Call; kein Consent-Kanal dort → bewusst fail-closed.ipc-handlers.tsverliert seine ~180-Zeilen-Inline-Kette und ruft die geteilten Phasen (guardInput vor Commands, guardDispatch danach — gleiche Logik, gleiche Reihenfolge, kein Verhaltenswechsel). Die IPC-Oberfläche ist byte-identisch erhalten:message:error/message:permission-required/message:safety-warningund dieinjectionBlocked/sensitiveDataBlocked/pendingConsent-Returns; die volleUploadPermissionResponse+ Request werden an den Consent-Dialog durchgereicht.Tests
request-guard.test.ts: 18 Tests — preflightGuard (non-interactive) + guardInput/guardDispatch (Chat-Phasen: Consent-Surfacing, Routing-sensitivity/reasons, Budget-Fallback/Block).GuardDeps-Mock (testen Logik, nicht den Guard).tsc -p tsconfig.main.jsonclean.Review-Punkte
Dokumentation
Security-Audit (2026-08-21), Mingly rls-gap Medium. Refactor-Plan §5 aus dem Handoff — inkl. des dort empfohlenen Chat-Pfad-Dedups.
🤖 Generated with Claude Code