Open
Conversation
Core detection and scanning infrastructure: - PII detection (email, phone, SSN, IP, credit cards) - Secrets detection (API keys, tokens, connection strings) - Prompt injection detection with structural analysis - Invisible character and encoding attack detection - Malicious URL detection and dangerous code patterns - Output injection detection for AI responses - Security event logging system - Support for warn, sanitize, and block action modes Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Settings infrastructure: - LLM Guard settings tab in Settings Modal - Settings persistence in settingsStore - Configuration import/export functionality - Per-session security policies support - Custom regex patterns configuration - TypeScript types for security features Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Visual components for security features: - LlmGuardIndicator with real-time status - SecurityBadge for session list indicators - SecurityEventsPanel for Right Bar - ScanProgressIndicator for input feedback - SanitizedContentDiff for visual diff view - FindingDetails for security findings - SensitiveContentOverlay for input highlighting - SecurityRecommendationsPanel - SessionSecurityModal for per-session policies Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Integration layer: - useSensitiveContentDetection hook for real-time preview - useSecurityToasts hook for notifications - Keyboard shortcuts for security features - Group Chat inter-agent protection - Security recommendations system - App.tsx and store integration Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Test suites for: - Security IPC handler and preload API - Output injection detection - Security recommendations system - Config export functionality - Security logger - UI components (LlmGuardIndicator, SecurityBadge, etc.) - Hooks (useSecurityToasts) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Comprehensive LLM Guard security documentation - Documentation link in settings UI - Updated features documentation - Package dependencies Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add spell-check IPC handlers and preload API for Electron's built-in spell checker: - Get system locale for language detection - Add words to custom dictionary - Set/get spell-checker languages - Get available dictionaries Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a full LLM Guard subsystem: scanner modules, config import/export, custom patterns, vaulting, event logger & recommendations, IPC + preload APIs, spellcheck handlers, process-manager integration (pre/post/inter-agent), renderer UI (indicator, badge, events panel, diffs), scripts, docs, and extensive tests. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes Possibly related issues
Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Merged upstream's single-parse refactor with LLM Guard output scanning. The upstream change parses JSON once and uses if/else instead of try/catch. The LLM Guard branch applies output guards to non-JSON lines. Combined approach: use the if/else pattern from upstream while maintaining LLM Guard scanning on the else branch for raw output lines. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Greptile SummaryThis PR introduces LLM Guard, a comprehensive client-side security layer for Maestro that scans both input prompts and LLM output responses for sensitive content, including PII, secrets, prompt injection attempts, malicious URLs, and dangerous code patterns. The feature integrates deeply into the process lifecycle — pre-scan occurs before a prompt is sent to the agent, and post-scan is applied to every chunk of agent output in Key additions:
Issues found:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant R as Renderer
participant IPC as IPC Handler (process.ts)
participant Guard as LLM Guard (index.ts)
participant Vault as PII Vault
participant Logger as Security Logger
participant PM as Process Manager
participant Agent as AI Agent Process
participant StdoutH as StdoutHandler
R->>IPC: process:spawn (prompt)
IPC->>Guard: runLlmGuardPre(prompt, config)
Guard->>Guard: detectInvisibleChars / checkBannedContent
Guard->>Guard: redactSecrets
Guard->>Guard: scanUrls / applyCustomPatterns
Guard->>Vault: anonymizePii → store originals
Guard->>Guard: detectPromptInjection / analyzePromptStructure
Guard-->>IPC: LlmGuardPreResult (sanitizedPrompt, vault, findings)
IPC->>Logger: logSecurityEvent(input_scan, findings) → persists to JSONL ⚠️
IPC->>R: security:event (findingTypes only, no values)
IPC->>PM: spawn(sanitizedPrompt, llmGuardState)
PM->>Agent: child_process.spawn
Agent-->>StdoutH: stdout data
StdoutH->>Guard: runLlmGuardPost(response, vault, config)
Guard->>Guard: deanonymizePii (restore from Vault)
Guard->>Guard: redactSecrets / detectPiiLeakage
Guard->>Guard: scanCode / detectOutputInjection / scanUrls
Guard-->>StdoutH: LlmGuardPostResult (sanitizedResponse, findings)
StdoutH->>Logger: logSecurityEvent(output_scan, findings) → persists to JSONL ⚠️
StdoutH->>R: security-event (findingTypes only)
StdoutH->>R: emitDataBuffered(sanitizedResponse)
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+155
to
+158
| async function appendEventToFile(event: SecurityEvent): Promise<void> { | ||
| const filePath = getSecurityEventsPath(); | ||
| const line = JSON.stringify(event) + '\n'; | ||
| await fs.appendFile(filePath, line, 'utf-8'); |
There was a problem hiding this comment.
Sensitive values are stored in plaintext on disk. The SecurityEvent.findings array contains LlmGuardFinding[] objects, each with a value field holding the actual matched sensitive text (raw API keys, credit card numbers, PII strings, etc.). These events are serialized in full and appended to ~/.maestro/security-events.jsonl.
For example, if a GitHub PAT is detected, the audit log will contain:
{ "type": "SECRET_GITHUB_TOKEN", "value": "ghp_ACTUAL_TOKEN_HERE", ... }This means the very secrets that LLM Guard is designed to protect against sending to the LLM are now stored verbatim in a local plaintext file and are also sent back to the renderer via the security:events:get IPC handler. Any process with read access to the user's data directory (or any compromised renderer) can extract the plain-text secrets from the log.
Consider scrubbing or truncating finding.value before persisting — for instance, replacing it with a short prefix+suffix excerpt (e.g. "ghp_ABCD...XYZ") or a fixed placeholder so the type is still auditable without storing the full secret.
Comment on lines
+2468
to
+2477
| while ((match = matcher.exec(text)) !== null) { | ||
| const value = match[0]; | ||
| findings.push({ | ||
| type, | ||
| value, | ||
| start: match.index, | ||
| end: match.index + value.length, | ||
| confidence, | ||
| }); | ||
| } |
There was a problem hiding this comment.
collectMatches has no protection against zero-length regex matches. If a pattern matches an empty string, exec will not advance lastIndex and the loop will hang indefinitely.
This guard is already present elsewhere in the file (e.g. in detectOutputInjection at lines 1693-1696):
if (value.length === 0) {
matcher.lastIndex++;
continue;
}The same guard should be added to collectMatches:
Suggested change
| while ((match = matcher.exec(text)) !== null) { | |
| const value = match[0]; | |
| findings.push({ | |
| type, | |
| value, | |
| start: match.index, | |
| end: match.index + value.length, | |
| confidence, | |
| }); | |
| } | |
| while ((match = matcher.exec(text)) !== null) { | |
| const value = match[0]; | |
| // Guard against zero-length matches to prevent infinite loops | |
| if (value.length === 0) { | |
| matcher.lastIndex++; | |
| continue; | |
| } | |
| findings.push({ | |
| type, | |
| value, | |
| start: match.index, | |
| end: match.index + value.length, | |
| confidence, | |
| }); | |
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/process-manager/handlers/StdoutHandler.ts (1)
260-266:⚠️ Potential issue | 🟡 MinorRemove debug
console.logstatements before merging.These debug logs (also at Lines 283-286, 469-474, 483-486) appear to be development artifacts and should be removed or converted to proper logger calls before merging.
// DEBUG: Log usage extracted from parser console.log('[StdoutHandler] Usage from parser (line 255 path)', {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/process-manager/handlers/StdoutHandler.ts` around lines 260 - 266, Remove the development console.log debug statements in the StdoutHandler class (the statements starting with "[StdoutHandler] Usage from parser (line 255 path)" and the similar messages around the other occurrences) and either delete them or replace them with proper logger calls (e.g., use the existing logger instance such as this.logger.debug or processLogger.debug) inside the relevant method (StdoutHandler's stdout handling method) so debug output uses the project's logging system instead of console.log before merging.src/main/group-chat/group-chat-router.ts (2)
648-650:⚠️ Potential issue | 🔴 CriticalThe unsanitized moderator message is still fed back into participant prompts.
You append
messageto the shared log before the inter-agent scan, then rebuildhistoryContextfrom that log. Even whensanitizedMessagediffers or the message is blocked for a participant, the newest[moderator]: ...entry still contains the original content, so the guard is bypassed via{{HISTORY_CONTEXT}}.Suggested fix
- const chatHistory = await readLog(updatedChat.logPath); - const historyContext = chatHistory - .slice(-15) - .map( - (m) => `[${m.from}]: ${m.content.substring(0, 500)}${m.content.length > 500 ? '...' : ''}` - ) - .join('\n'); + const chatHistory = await readLog(updatedChat.logPath); @@ - const participantPrompt = groupChatParticipantRequestPrompt + const historyContextForParticipant = chatHistory + .slice(-15, -1) + .concat([{ from: 'moderator', content: sanitizedMessage }]) + .map( + (m) => + `[${m.from}]: ${m.content.substring(0, 500)}${m.content.length > 500 ? '...' : ''}` + ) + .join('\n'); + + const participantPrompt = groupChatParticipantRequestPrompt .replace(/\{\{PARTICIPANT_NAME\}\}/g, participantName) .replace(/\{\{GROUP_CHAT_NAME\}\}/g, updatedChat.name) .replace(/\{\{READ_ONLY_NOTE\}\}/g, readOnlyNote) .replace(/\{\{GROUP_CHAT_FOLDER\}\}/g, groupChatFolder) - .replace(/\{\{HISTORY_CONTEXT\}\}/g, historyContext) + .replace(/\{\{HISTORY_CONTEXT\}\}/g, historyContextForParticipant) .replace(/\{\{READ_ONLY_LABEL\}\}/g, readOnlyLabel) .replace(/\{\{MESSAGE\}\}/g, sanitizedMessage) .replace(/\{\{READ_ONLY_INSTRUCTION\}\}/g, readOnlyInstruction);Also applies to: 794-913
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/group-chat/group-chat-router.ts` around lines 648 - 650, The current flow appends the raw moderator `message` to the shared log via `appendToLog(chat.logPath, 'moderator', message)` before running the inter-agent scan and rebuilding `historyContext`, which lets unsanitized content leak into `{{HISTORY_CONTEXT}}`; change the logic so you only append the sanitized or redacted content (e.g., `sanitizedMessage` or a blocked-placeholder) to the log, or defer calling `appendToLog` until after the inter-agent scan decision, and ensure the rebuilt `historyContext` is always sourced from the sanitized/blocked entry; update all occurrences (including the block at the lines referenced and the other similar block around the 794-913 range) to use the sanitized value when calling `appendToLog` and when regenerating `historyContext`.
787-887:⚠️ Potential issue | 🟠 MajorReset the chat state when mentions exist but none are actually spawned.
The idle cleanup only runs when
mentions.length === 0. If every mentioned participant is blocked, unavailable, or fails to spawn,participantsToRespondstays empty, but the function leaves the group chat in its previous busy state and keeps the power-block reason around.Suggested fix
// Store pending participants for synthesis tracking if (participantsToRespond.size > 0) { pendingParticipantResponses.set(groupChatId, participantsToRespond); console.log( `[GroupChat:Debug] Waiting for ${participantsToRespond.size} participant(s) to respond: ${[...participantsToRespond].join(', ')}` ); // Set state to show agents are working groupChatEmitters.emitStateChange?.(groupChatId, 'agent-working'); console.log(`[GroupChat:Debug] Emitted state change: agent-working`); + } else if (mentions.length > 0) { + groupChatEmitters.emitStateChange?.(groupChatId, 'idle'); + powerManager.removeBlockReason(`groupchat:${groupChatId}`); }Also applies to: 1055-1073
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/group-chat/group-chat-router.ts` around lines 787 - 887, When mentions exist but none of them actually spawn (participantsToRespond is empty), reset the group chat busy/power-block state: after the participant-spawning loop check if mentions.length > 0 && participantsToRespond?.length === 0 and then emit idle state for the moderator and all mentioned participants via groupChatEmitters.emitParticipantState?.(groupChatId, participantName, 'idle') (and emitParticipantState for 'Moderator'), and clear any stored power-block reason using the existing API (e.g. call clearPowerBlockReasonCallback?.(groupChatId) or set the stored powerBlockReason on the group chat state to null/undefined). Ensure you reference groupChatId, mentions, participantsToRespond and groupChatEmitters.emitParticipantState when adding this cleanup.
🟠 Major comments (23)
src/__tests__/main/process-manager/handlers/ExitHandler.test.ts-234-236 (1)
234-236:⚠️ Potential issue | 🟠 MajorUse a non-triggering fixture for the GitHub token.
The PAT-shaped literal at Lines 234-236 is likely to keep secret scanning noisy or failing. Build the token from pieces and reuse that variable when composing
resultJson.Safer fixture
- const githubToken = 'ghp_abcdefghijklmnopqrstuvwxyz1234567890'; - const resultJson = - '{"type":"result","text":"Reply to [EMAIL_1] and remove ghp_abcdefghijklmnopqrstuvwxyz1234567890"}'; + const githubToken = ['ghp_', 'abcdefghijklmnopqrstuvwxyz1234567890'].join(''); + const resultJson = JSON.stringify({ + type: 'result', + text: `Reply to [EMAIL_1] and remove ${githubToken}`, + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/process-manager/handlers/ExitHandler.test.ts` around lines 234 - 236, The test currently embeds a PAT-shaped literal in githubToken and resultJson; instead, construct githubToken by concatenating safe string pieces (e.g., 'ghp_' + 'abcdefghijklmnopqrstuvwxyz' + '1234567890') and then reference that githubToken variable when building resultJson (use the githubToken variable inside the string composition for resultJson) so the test no longer contains a single PAT-shaped literal; update any assertions that rely on resultJson accordingly.src/__tests__/main/process-manager/handlers/StdoutHandler.test.ts-228-242 (1)
228-242:⚠️ Potential issue | 🟠 MajorAvoid PAT-shaped fixtures and assert that nothing unsanitized was emitted.
Line 231 is already tripping secret scanning, and the two positive-only
toHaveBeenCalledWith(...stringContaining())checks would still pass if the raw token or[EMAIL_1]leaked in an earlier chunk. Build the token from pieces and assert across all emitted payloads that the raw token and placeholder never reach the renderer.Suggested test hardening
+ const githubToken = ['ghp_', '123456789012345678901234567890123456'].join(''); sendJsonLine(handler, sessionId, { type: 'result', - result: - 'Contact [EMAIL_1] and rotate ghp_123456789012345678901234567890123456 immediately.', + result: `Contact [EMAIL_1] and rotate ${githubToken} immediately.`, }); - expect(bufferManager.emitDataBuffered).toHaveBeenCalledWith( - sessionId, - expect.stringContaining('john@example.com') - ); - expect(bufferManager.emitDataBuffered).toHaveBeenCalledWith( - sessionId, - expect.stringContaining('[REDACTED_SECRET_GITHUB_TOKEN_1]') - ); + const emittedPayloads = (bufferManager.emitDataBuffered as any).mock.calls.map( + (call: any[]) => String(call[1]) + ); + expect(emittedPayloads.some((payload) => payload.includes('john@example.com'))).toBe(true); + expect( + emittedPayloads.some((payload) => + payload.includes('[REDACTED_SECRET_GITHUB_TOKEN_1]') + ) + ).toBe(true); + expect(emittedPayloads.some((payload) => payload.includes('[EMAIL_1]'))).toBe(false); + expect(emittedPayloads.some((payload) => payload.includes(githubToken))).toBe(false);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/process-manager/handlers/StdoutHandler.test.ts` around lines 228 - 242, The test uses a PAT-shaped literal and only positive contains checks so it can both trip scanners and miss leaks; change the fixture to build the token from concatenated pieces (instead of a single ghp_... literal) when calling sendJsonLine(handler, sessionId, ...) and replace the two positive expect.stringContaining assertions with a negative assertion over all emitted payloads: gather bufferManager.emitDataBuffered.mock.calls (or inspect proc/emitted output) and assert that none of the joined payload strings include the raw token or the raw placeholder "[EMAIL_1]" (use toEqual(false)/not.toContain or equivalent negative checks) so the test ensures no unsanitized secrets reach the renderer while avoiding PAT-shaped literals in the source.src/main/app-lifecycle/window-manager.ts-127-135 (1)
127-135:⚠️ Potential issue | 🟠 MajorPass only supported locales to
setSpellCheckerLanguages().
session.setSpellCheckerLanguages()will error if any language code is not inavailableSpellCheckerLanguages. Sinceapp.getLocale()can return unsupported variants (e.g.,"de"or unsupported"en-*"locales), the current fallback logic leaves unsupported locales without a safe fallback path. Filter againstavailableSpellCheckerLanguagesbefore callingsetSpellCheckerLanguages():Suggested implementation
const systemLocale = app.getLocale(); - const spellCheckLanguages = [systemLocale]; - // Add fallback to generic English if not already included - if (!systemLocale.startsWith('en')) { - spellCheckLanguages.push('en-US'); - } - mainWindow.webContents.session.setSpellCheckerLanguages(spellCheckLanguages); + const available = new Set(mainWindow.webContents.session.availableSpellCheckerLanguages); + const candidates = [systemLocale, systemLocale.split('-')[0], 'en-US']; + const spellCheckLanguages = candidates.filter( + (lang, index) => !!lang && available.has(lang) && candidates.indexOf(lang) === index + ); + + if (spellCheckLanguages.length > 0) { + mainWindow.webContents.session.setSpellCheckerLanguages(spellCheckLanguages); + } else { + logger.warn(`No supported spell-check dictionary found for locale ${systemLocale}`, 'Window'); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/app-lifecycle/window-manager.ts` around lines 127 - 135, The code currently passes app.getLocale() directly into spellCheckLanguages and calls mainWindow.webContents.session.setSpellCheckerLanguages(), which can throw if values are not in availableSpellCheckerLanguages; update the logic to check app.getLocale() and the fallback against session.availableSpellCheckerLanguages (or availableSpellCheckerLanguages) and only include locales present in that list, e.g., build spellCheckLanguages by filtering [systemLocale] + fallback ('en-US') through availableSpellCheckerLanguages, and if the resulting array is empty use a safe default or skip calling mainWindow.webContents.session.setSpellCheckerLanguages(); change references in the function that set spellCheckLanguages and the call to setSpellCheckerLanguages accordingly.src/main/process-manager/handlers/StderrHandler.ts-47-58 (1)
47-58:⚠️ Potential issue | 🟠 MajorPersist stderr guard findings through the security-event pipeline.
This path only writes to
logger.warn(). When Codex output is sanitized or blocked on stderr, those findings never reach the security event store, so the Security Events UI and recommendations will miss output that was actually filtered.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/process-manager/handlers/StderrHandler.ts` around lines 47 - 58, The stderr guard branch currently only logs findings via logger.warn after calling runLlmGuardPost; update the blocked/sanitized path to also persist these findings to the security event pipeline by invoking the project's security event emitter/store (e.g., emitSecurityEvent or securityEventStore.persist) with the guardResult and context (sessionId, managedProcess.toolType, guardResult.findings, guardResult.blocked, guardResult.blockReason) so that LLMGuard findings from StderrHandler are recorded in the Security Events UI and recommendations pipeline in addition to the logger.warn call.src/__tests__/renderer/components/FindingDetails.test.ts-16-131 (1)
16-131:⚠️ Potential issue | 🟠 MajorExercise
FindingDetailsinstead of a copied implementation.These helpers duplicate the component logic inside the test file, so the suite can stay green even when
FindingDetails.tsxregresses. Import the real helpers, or render the component and assert on its output, so the test covers production behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/FindingDetails.test.ts` around lines 16 - 131, The test contains duplicated helper implementations (formatFindingType, maskValue, getConfidenceLabel, getDocUrlForType, getCategoryType) which masks regressions in FindingDetails; replace these copies by importing the real helpers from the component module (or render FindingDetails and assert on rendered output) so the tests exercise production logic—locate the helper functions in FindingDetails.tsx and update the test to import and use them (or mount <FindingDetails /> and assert expected strings/colors/links) instead of the inline implementations.src/__tests__/renderer/components/SecurityBadge.test.ts-18-48 (1)
18-48:⚠️ Potential issue | 🟠 MajorThese tests are tautological; they never touch
SecurityBadge.The file mirrors the component logic in local helpers and then asserts on those helpers/local variables, so the suite can stay green while the real component regresses. Please render the component and assert the DOM/timer behavior instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/SecurityBadge.test.ts` around lines 18 - 48, Tests currently mirror component logic via local helpers getStatus and getBadgeColor instead of exercising SecurityBadge; remove those tautological helpers and rewrite tests to render the actual SecurityBadge component (import SecurityBadge) with different prop combinations (visible, blockedCount, warningCount, scanCount), use RTL render to query the DOM (getByRole/getByText/getByTestId) to assert rendered status text, badge color (via computed style or class), and tooltip/timer behavior using jest.useFakeTimers() and advancing timers; ensure each case that previously used getStatus/getBadgeColor now renders SecurityBadge and asserts on DOM output and timer-driven changes.src/main/ipc/handlers/spellcheck.ts-68-68 (1)
68-68:⚠️ Potential issue | 🟠 MajorDon't persist raw spellcheck input in logs.
Both handlers log the exact user-provided word. Misspellings and custom dictionary entries can contain names, secrets, or proprietary terms, so this creates avoidable sensitive-data retention in main-process logs.
🛡️ Safer logging
- logger.debug(`Spell-check suggestions requested for: ${word}`, LOG_CONTEXT); + logger.debug('Spell-check suggestions requested', LOG_CONTEXT, { + length: word.length, + }); - logger.info(`Added word to spell-checker dictionary: ${word}`, LOG_CONTEXT); + logger.info('Added word to spell-checker dictionary', LOG_CONTEXT, { + length: word.length, + });Also applies to: 81-81
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/ipc/handlers/spellcheck.ts` at line 68, Remove raw user input from logs by stopping use of the interpolated variable in the debug call (logger.debug(`Spell-check suggestions requested for: ${word}`, LOG_CONTEXT)) and the similar call at the other occurrence; instead log a non-sensitive representation such as a fixed placeholder ("[redacted]"), a length/character-count, or a stable one-way digest (e.g., sha256 of word) created via the crypto module before logging. Update the spellcheck handler(s) in spellcheck.ts to compute the sanitized value (placeholder/length/hash) and pass that into logger.debug with the same context, ensuring no raw word is ever written to the main-process logs.src/__tests__/setup.ts-542-550 (1)
542-550:⚠️ Potential issue | 🟠 MajorKeep
window.maestro.securityaligned with the new security API.This mock still only covers the event-list subset. The export/config/recommendation/preview methods added in this PR will be
undefinedin renderer tests, so those paths will fail as soon as they're exercised.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/setup.ts` around lines 542 - 550, The window.maestro.security mock only covers event-list methods and needs to include the new API surface (export, config, recommendation, preview methods) so renderer tests don't see undefined; update the security mock object (window.maestro.security) to add vi.fn() mocks for each new method introduced in this PR (e.g., export*, getConfig/getSettings, recommend*/getRecommendations, preview*/generatePreview or similarly named functions) and return sensible defaults or resolved promises (mockResolvedValue) matching the real API shapes so tests exercise those paths without throwing.src/renderer/components/AppModals.tsx-2400-2400 (1)
2400-2400:⚠️ Potential issue | 🟠 MajorForward
promptLlmGuardEnabledintoAppUtilityModals.
AppModalsaccepts/destructures the new flag here, but the<AppUtilityModals />call never passes it through. In the normalAppModalspath,PromptComposerModaltherefore always seesundefined, so the new guard toggle stays off.🐛 Minimal fix
promptTabShowThinking={promptTabShowThinking} onPromptToggleTabShowThinking={onPromptToggleTabShowThinking} promptSupportsThinking={promptSupportsThinking} promptEnterToSend={promptEnterToSend} onPromptToggleEnterToSend={onPromptToggleEnterToSend} + promptLlmGuardEnabled={promptLlmGuardEnabled} queueBrowserOpen={queueBrowserOpen}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/AppModals.tsx` at line 2400, AppModals is destructuring promptLlmGuardEnabled but not passing it to AppUtilityModals, so PromptComposerModal receives undefined; update the JSX call to <AppUtilityModals ...> to forward the promptLlmGuardEnabled prop (prop name promptLlmGuardEnabled) through to AppUtilityModals so it can propagate to PromptComposerModal (or any downstream component that expects promptLlmGuardEnabled).src/main/process-manager/handlers/ExitHandler.ts-65-86 (1)
65-86:⚠️ Potential issue | 🟠 MajorEmit the realtime security event before awaiting persistence.
logSecurityEvent()is fire-and-forget with only a.then().handleExit()keeps going, emits'exit', and deletes the process immediately afterward, so the lastsecurity-eventcan arrive after teardown or be dropped entirely; a rejection is also currently unhandled.⏱️ Safer ordering
- logSecurityEvent({ - sessionId, - tabId: managedProcess.tabId, - eventType: guardResult.blocked ? 'blocked' : 'output_scan', - findings: guardResult.findings, - action, - originalLength: resultText.length, - sanitizedLength: guardResult.sanitizedResponse.length, - }).then((event) => { - // Emit to ProcessManager listeners for real-time UI forwarding - const eventData: SecurityEventData = { - sessionId: event.sessionId, - tabId: event.tabId, - eventType: event.eventType, - findingTypes: event.findings.map((f) => f.type), - findingCount: event.findings.length, - action: event.action, - originalLength: event.originalLength, - sanitizedLength: event.sanitizedLength, - }; - this.emitter.emit('security-event', eventData); - }); + const eventData: SecurityEventData = { + sessionId, + tabId: managedProcess.tabId, + eventType: guardResult.blocked ? 'blocked' : 'output_scan', + findingTypes: guardResult.findings.map((f) => f.type), + findingCount: guardResult.findings.length, + action, + originalLength: resultText.length, + sanitizedLength: guardResult.sanitizedResponse.length, + }; + this.emitter.emit('security-event', eventData); + + void logSecurityEvent({ + sessionId, + tabId: managedProcess.tabId, + eventType: eventData.eventType, + findings: guardResult.findings, + action, + originalLength: resultText.length, + sanitizedLength: eventData.sanitizedLength, + }).catch((error) => { + logger.error('[LLMGuard] Failed to persist security event', 'LLMGuard', { + sessionId, + error: String(error), + }); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/process-manager/handlers/ExitHandler.ts` around lines 65 - 86, The current logic calls logSecurityEvent(...).then(...) as fire-and-forget inside handleExit, causing the emitter.emit('security-event', ...) to run after process teardown and leaving rejections unhandled; change handleExit so it emits the realtime security event to this.emitter synchronously (use the event data built from guardResult/findings and this.emitter.emit('security-event', eventData)) before awaiting or calling logSecurityEvent, then call logSecurityEvent(...) and await it (or at least attach .catch to handle errors) to ensure persistence completes or failures are handled before the process is deleted; reference the logSecurityEvent call, the eventData construction, this.emitter.emit('security-event', ...), and the handleExit flow that deletes the managedProcess.src/renderer/components/FindingDetails.tsx-244-249 (1)
244-249:⚠️ Potential issue | 🟠 MajorMask all secret-like findings before rendering them.
getIconForType()andgetCategoryColor()already classify*KEY*findings as secrets, butmaskValue()and the reveal toggle only gate onSECRET_,PASSWORD,TOKEN, andPII_.API_KEY-style findings will therefore render raw values with no explicit reveal step.Also applies to: 416-417
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/FindingDetails.tsx` around lines 244 - 249, maskValue only checks for SECRET_, PASSWORD, TOKEN, and PII_ so KEY-style findings (e.g., API_KEY) are not masked; update the logic by adding KEY detection (e.g., include type.includes('KEY') or a regex like /\bKEY\b/) or better yet extract a shared helper isSecretType(type: string) used by getIconForType()/getCategoryColor(), maskValue, and the reveal-toggle gating so all secret-like types (SECRET_, KEY, PASSWORD, TOKEN, PII_) are consistently treated as secrets before rendering.src/__tests__/renderer/components/SecurityEventsPanel.test.ts-28-95 (1)
28-95:⚠️ Potential issue | 🟠 MajorThese tests aren't exercising the production export logic.
buildExportFilters(),generateExportFilename(),getMimeType(), andtoggleEventType()are reimplemented locally here instead of imported fromsrc/renderer/components/SecurityEventsPanel.tsx. That means this suite can stay green even if the real panel logic regresses.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/SecurityEventsPanel.test.ts` around lines 28 - 95, The tests reimplement production helpers (buildExportFilters, generateExportFilename, getMimeType, toggleEventType) instead of using the real implementations, so replace the local helper copies in SecurityEventsPanel.test.ts with imports of the actual functions from the SecurityEventsPanel module (remove the local definitions and add import { buildExportFilters, generateExportFilename, getMimeType, toggleEventType } from the component module); ensure the imported function names match the module exports and update any test usages to reference the imported symbols so the test exercises the real production logic.src/__tests__/main/security/llm-guard/recommendations.test.ts-419-419 (1)
419-419:⚠️ Potential issue | 🟠 MajorMake these
forEachcallbacks explicitlyvoid.Biome flags both expression-body callbacks because they return the value from
expect(...). Wrap the assertions in braces, or switch tofor...of.Suggested fix
- highOnly.forEach((r) => expect(r.severity).toBe('high')); + highOnly.forEach((r) => { + expect(r.severity).toBe('high'); + }); @@ - secretsOnly.forEach((r) => expect(r.category).toBe('secret_detection')); + secretsOnly.forEach((r) => { + expect(r.category).toBe('secret_detection'); + });Also applies to: 442-442
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/security/llm-guard/recommendations.test.ts` at line 419, The forEach callbacks return the expect(...) value which Biome flags; update the callbacks to be explicitly void by changing expression-body arrow functions to block-body arrow functions (e.g., replace highOnly.forEach((r) => expect(r.severity).toBe('high')) with a block that calls expect and returns nothing) and do the same for the other forEach at the same test (e.g., lowOnly.forEach). Ensure you modify the callbacks for highOnly.forEach and the analogous lowOnly.forEach so they use block bodies (or for...of) to avoid returning the expect result.src/__tests__/renderer/components/SanitizedContentDiff.test.tsx-89-94 (1)
89-94:⚠️ Potential issue | 🟠 MajorReplace the secret-shaped fixture before CI flags it.
sk-1234567890abcdefis already matching Gitleaks. Even as test data, this will be treated like a leaked credential by the pipeline.Suggested fix
- const originalContent = 'My email is test@example.com and my API key is sk-1234567890abcdef'; + const fakeApiKey = ['sk', '1234567890abcdef'].join('-'); + const originalContent = `My email is test@example.com and my API key is ${fakeApiKey}`; const sanitizedContent = 'My email is [EMAIL_REDACTED] and my API key is [API_KEY_REDACTED]'; const findings: Finding[] = [ createFinding('PII_EMAIL', 'test@example.com', 12, 28, 0.98, '[EMAIL_REDACTED]'), - createFinding('SECRET_API_KEY', 'sk-1234567890abcdef', 48, 67, 0.99, '[API_KEY_REDACTED]'), + createFinding('SECRET_API_KEY', fakeApiKey, 48, 67, 0.99, '[API_KEY_REDACTED]'), ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/SanitizedContentDiff.test.tsx` around lines 89 - 94, Replace the secret-shaped API key fixture with a non-sensitive placeholder across the test: update originalContent (e.g., use "sk-REDACTED-1234" or "sk-xxxxxxxxxxxx" instead of "sk-1234567890abcdef"), update sanitizedContent to match the placeholder's redaction "[API_KEY_REDACTED]" if needed, update the findings array entry created by createFinding (raw value argument and start/end offsets) to reflect the new placeholder length and offsets, and ensure the confidence value and replacement text remain the same; adjust any indices used by createFinding so the test offsets remain accurate.src/__tests__/renderer/components/SecurityEventsPanel.test.ts-43-49 (1)
43-49:⚠️ Potential issue | 🟠 MajorParse custom date ranges in local time.
The custom date export feature in both the test helper and the actual component uses
new Date('YYYY-MM-DD'), which parses as UTC midnight. In non-UTC time zones, this causes the date to shift backward beforesetHours(...)runs. The test assertions at lines 137-150 will fail in any timezone that's not UTC, making the tests timezone-dependent and the export filters potentially incorrect for users in non-UTC zones.Suggested fix
} else if (dateRange === 'custom') { if (customStartDate) { - filters.startDate = new Date(customStartDate).getTime(); + const [year, month, day] = customStartDate.split('-').map(Number); + filters.startDate = new Date(year, month - 1, day, 0, 0, 0, 0).getTime(); } if (customEndDate) { - filters.endDate = new Date(customEndDate).setHours(23, 59, 59, 999); + const [year, month, day] = customEndDate.split('-').map(Number); + filters.endDate = new Date(year, month - 1, day, 23, 59, 59, 999).getTime(); } }Apply the same fix to lines 586-589 in
src/renderer/components/SecurityEventsPanel.tsx.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/renderer/components/SecurityEventsPanel.test.ts` around lines 43 - 49, The test and component parse customStartDate/customEndDate with new Date('YYYY-MM-DD') which yields UTC midnight and causes timezone shifts; change parsing to construct local dates (e.g., split the YYYY-MM-DD string into year, month, day and call new Date(year, month-1, day)) and then set filters.startDate = localDate.getTime() and filters.endDate = new Date(year, month-1, day).setHours(23,59,59,999) to produce local start/end timestamps; update uses in the test helper (where dateRange, customStartDate, customEndDate, filters.startDate, filters.endDate are set) and make the identical change in the SecurityEventsPanel component (the same variables/assignments) so both test and runtime use local-time parsing.src/main/ipc/handlers/process.ts-255-267 (1)
255-267:⚠️ Potential issue | 🟠 MajorDon't persist prompt text in process logs.
Post-sanitization is not a privacy boundary. Anything the detectors miss still gets written to logs here (
first50/last50and up to 500 chars ofprompt), which undercuts the goal of reducing sensitive-data exposure.Also applies to: 390-409
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/ipc/handlers/process.ts` around lines 255 - 267, The current Windows-only logging block uses effectivePrompt and writes actual prompt snippets (first50/last50) via logFn which can leak sensitive content; change the log to never include any substring of effectivePrompt or prompt text—replace the promptPreview object with non-sensitive metadata such as promptLength: effectivePrompt.length and boolean flags (containsHash, containsNewline, maybe containsLongLines) and keep sessionId and LOG_CONTEXT; ensure logged fields reference effectivePrompt only to compute booleans/length and do not persist any characters from the prompt.src/main/preload/security.ts-15-24 (1)
15-24:⚠️ Potential issue | 🟠 MajorPreload typings still exclude
inter_agent_scan.This PR now produces
inter_agent_scanevents, andExportFilterOptionsalready accepts that value, butSecurityEventData,SecurityEvent, andgetEventsByType()do not. Renderer code therefore cannot request or type those events correctly.Suggested fix
- eventType: 'input_scan' | 'output_scan' | 'blocked' | 'warning' | 'scan_start' | 'scan_complete'; + eventType: + | 'input_scan' + | 'output_scan' + | 'blocked' + | 'warning' + | 'scan_start' + | 'scan_complete' + | 'inter_agent_scan'; @@ - getEventsByType: ( - eventType: 'input_scan' | 'output_scan' | 'blocked' | 'warning', + getEventsByType: ( + eventType: + | 'input_scan' + | 'output_scan' + | 'blocked' + | 'warning' + | 'inter_agent_scan', limit?: number ): Promise<SecurityEvent[]> =>Also applies to: 41-58, 96-107, 265-269
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/preload/security.ts` around lines 15 - 24, The SecurityEvent-related typings are missing the 'inter_agent_scan' event variant; update the union types and any affected type definitions so renderer code can request and type these events. Specifically, add 'inter_agent_scan' to the eventType union in SecurityEventData, include it in the SecurityEvent discriminated union (where eventType is declared), and update getEventsByType()'s allowed event type parameter/return typing to accept 'inter_agent_scan'; ensure any places that build or narrow on eventType (e.g., in pattern matches or mapped types) include the new value so TypeScript recognizes inter_agent_scan events across SecurityEventData, SecurityEvent, and getEventsByType.src/renderer/components/SanitizedContentDiff.tsx-95-98 (1)
95-98:⚠️ Potential issue | 🟠 MajorTreat empty-string replacements as real sanitizations.
All of these checks use truthiness, so a finding with
replacement: ''is dropped from the diff, the legend, and even the “No sanitization changes” gate. That makes deletions and invisible-character removals disappear from the audit UI.Suggested fix
- if (!finding.replacement) continue; + if (finding.replacement === undefined) continue; @@ - const sortedFindings = sortFindingsByPosition(findings).filter((f) => f.replacement); + const sortedFindings = sortFindingsByPosition(findings).filter( + (f) => f.replacement !== undefined + ); @@ - const sortedFindings = sortFindingsByPosition(findings).filter((f) => f.replacement); + const sortedFindings = sortFindingsByPosition(findings).filter( + (f) => f.replacement !== undefined + ); @@ - const sanitizedFindings = findings.filter((f) => f.replacement); + const sanitizedFindings = findings.filter((f) => f.replacement !== undefined); @@ - if (f.replacement) types.add(f.type); + if (f.replacement !== undefined) types.add(f.type); @@ - if (findings.filter((f) => f.replacement).length === 0) { + if (findings.filter((f) => f.replacement !== undefined).length === 0) {Also applies to: 136-140, 201-205, 270-270, 500-509
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/SanitizedContentDiff.tsx` around lines 95 - 98, The code currently treats replacement values using truthiness so empty-string replacements are ignored; update the checks that skip findings to explicitly test for null/undefined (e.g., replace truthy checks like "if (!finding.replacement) continue" with an explicit null/undefined check such as "if (finding.replacement == null) continue") so that empty string ("") is considered a real replacement; apply this change to the loops/conditions involving sortedFindings and other checks in SanitizedContentDiff.tsx (the places that currently use !finding.replacement for filtering, legend rendering, the "No sanitization changes" gate, and similar blocks).src/renderer/components/SanitizedContentDiff.tsx-90-128 (1)
90-128:⚠️ Potential issue | 🟠 MajorNormalize overlapping findings before building segments.
These builders assume every finding range is disjoint. When two findings overlap,
currentPosadvances past the first match but the later overlapping match is still rendered, so the diff duplicates text and the sanitized-side offset math drifts. The scanners in this PR can emit overlapping findings, so the UI should merge or skip overlaps first.Also applies to: 148-185, 210-246
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/SanitizedContentDiff.tsx` around lines 90 - 128, The builders (e.g., buildOriginalSegments) assume findings are disjoint, but scanners can emit overlapping findings; fix by normalizing findings before segmentation: add a helper (e.g., normalizeFindings(sorted: Finding[]): Finding[]) called after sortFindingsByPosition that merges or drops overlaps deterministically (recommend merging contiguous/overlapping ranges into a single Finding with start = min, end = max, and a combined/first-choice replacement/type policy) so subsequent code in buildOriginalSegments (and sibling builders like buildSanitizedSegments) can iterate disjoint ranges without duplicating text or drifting offsets; replace uses of sortFindingsByPosition(findings) with normalizeFindings(sortFindingsByPosition(findings)).src/main/ipc/handlers/process.ts-206-240 (1)
206-240:⚠️ Potential issue | 🟠 MajorWarn-only scans are logged as
input_scan, so the renderer never sees a warning event.This branch only emits
blockedorinput_scan. Components likeLlmGuardIndicatorkey offeventType === 'warning', so warn-only findings will never increment the warning badge. Also, the fire-and-forgetlogSecurityEvent(...).then(...)has no.catch(), which turns logger I/O failures into unhandled rejections in the main process.Suggested fix
- logSecurityEvent({ + void logSecurityEvent({ sessionId: config.sessionId, tabId: config.tabId, - eventType: guardResult.blocked ? 'blocked' : 'input_scan', + eventType: guardResult.blocked + ? 'blocked' + : guardResult.warned + ? 'warning' + : 'input_scan', findings: guardResult.findings, action, originalLength: config.prompt!.length, sanitizedLength: guardResult.sanitizedPrompt.length, }).then((event) => { @@ }); } - }); + }).catch((error) => { + logger.error('Failed to log security event', LOG_CONTEXT, { + error, + sessionId: config.sessionId, + }); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/ipc/handlers/process.ts` around lines 206 - 240, The code logs warn-only LLM guard findings as 'input_scan' so the renderer never sees a 'warning' event and also leaves logSecurityEvent(...) promise unhandled; update the logic around llmGuardConfig/guardResult to set eventType to 'blocked' if guardResult.blocked, 'warning' if guardResult.warned, otherwise 'input_scan' (ensure the same value is passed into logSecurityEvent and emitted via mainWindow.webContents.send('security:event')), and attach a .catch(...) to the logSecurityEvent(...).then(...) chain to handle/log any errors (use getMainWindow(), isWebContentsAvailable(), and the returned event fields as currently used).src/main/security/llm-guard/custom-patterns.ts-45-57 (1)
45-57:⚠️ Potential issue | 🟠 MajorUser-supplied regexes are executed directly in the main process without ReDoS protection.
The file imports no regex-safety libraries despite
safe-regex2andsafe-regex-testbeing available in the project dependencies. Syntax validation invalidatePattern()cannot prevent catastrophic backtracking—a pattern like(a+)+$passes validation but will hangtestPattern()orapplyCustomPatterns()on inputs like a long string ofas followed by a non-matching character, turning a saved rule into an app-level DoS.Import and use one of the available safety libraries (
safe-regex2,safe-regex-test) in the validation and execution paths, or run pattern testing/matching in an isolated worker with a time budget.Also applies to: lines 68-98 (
testPattern), 108-160 (applyCustomPatterns)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/security/llm-guard/custom-patterns.ts` around lines 45 - 57, The current validatePattern, testPattern, and applyCustomPatterns functions run user regexes directly and are vulnerable to ReDoS; update these to use a regex-safety library (e.g., safe-regex2 or safe-regex-test) or run matches inside an isolated worker with a strict timeout: in validatePattern(pattern: string) call the chosen safety check (reject unsafe patterns and return a clear error), and in testPattern(...) and applyCustomPatterns(...) either (a) pre-validate the pattern with the safety check before compiling/executing or (b) move the actual RegExp construction and testing into a worker thread/process with a bounded time budget and propagate a timeout error back to the caller; ensure you reference and protect the code paths in validatePattern, testPattern, and applyCustomPatterns so unsafe or slow patterns are rejected or timed out.src/main/security/llm-guard/code-scanner.ts-250-259 (1)
250-259:⚠️ Potential issue | 🟠 Major
isInCodeBlock()can miss shell-prompt matches on the last line and after the first hit due to regex statefulness and incorrect EOF handling.
CODE_BLOCK_PATTERNS.shellPromptis a shared/gmregex. When used with.test(), it mutateslastIndex, causing subsequent calls to start from the wrong position. Additionally,text.indexOf('\n', start)returns-1at end-of-file, andsubstring(start, -1)treats the-1as0, resulting in an empty string whencurrentLineStart > 0. Together, these cause false negatives in shell-prompt detection, allowingcodeBlocksOnlyto silently drop dangerous commands.Suggested fix
- const currentLineStart = text.lastIndexOf('\n', start - 1) + 1; - const currentLine = text.substring(currentLineStart, text.indexOf('\n', start)); - if (CODE_BLOCK_PATTERNS.shellPrompt.test(currentLine)) { + const currentLineStart = text.lastIndexOf('\n', start - 1) + 1; + const currentLineEnd = text.indexOf('\n', start); + const currentLine = text.substring( + currentLineStart, + currentLineEnd === -1 ? text.length : currentLineEnd + ); + const shellPromptMatcher = new RegExp(CODE_BLOCK_PATTERNS.shellPrompt.source, 'm'); + if (shellPromptMatcher.test(currentLine)) { return true; }Also applies to: 289-292
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/security/llm-guard/code-scanner.ts` around lines 250 - 259, isInCodeBlock() misses shellPrompt matches because CODE_BLOCK_PATTERNS.shellPrompt is a shared /gm regex whose internal lastIndex is mutated by repeated .test() calls and because text.indexOf('\n', start) can return -1 causing substring(start, -1) to yield an empty string; fix by using a non-stateful regex (remove the global 'g' flag or create a new RegExp(CODE_BLOCK_PATTERNS.shellPrompt) per test) when checking CODE_BLOCK_PATTERNS.shellPrompt inside isInCodeBlock(), and change the line-end calculation to use end = text.indexOf('\n', start); if end === -1 then set end = text.length before extracting the current line (used by isInCodeBlock() and the related codeBlocksOnly logic around lines 289-292) so shell-prompt detection correctly handles the final line.src/main/security/llm-guard/index.ts-1631-1660 (1)
1631-1660:⚠️ Potential issue | 🟠 MajorUser-provided regex patterns in
banTopicsPatternsare vulnerable to ReDoS.The
banTopicsPatternsconfig allows users to define arbitrary regex patterns that are compiled and executed against input text. While syntax validation exists inconfig-export.ts, it only checks if the pattern compiles—it does not detect ReDoS vulnerabilities. A malicious or poorly-crafted pattern (e.g.,(a+)+$) could cause catastrophic backtracking and freeze the application.🛡️ Proposed fix: Add regex validation and timeout protection
// Check for banned topic patterns (regex, case-insensitive) if (config.banTopicsPatterns && config.banTopicsPatterns.length > 0) { for (const pattern of config.banTopicsPatterns) { if (!pattern || pattern.trim() === '') continue; try { + // Validate pattern complexity to prevent ReDoS + // Reject patterns with nested quantifiers or backreferences + const dangerousPatterns = /(\+|\*|\{[0-9,]+\})\s*(\+|\*|\?|\{)/; + if (dangerousPatterns.test(pattern)) { + console.warn(`Skipping potentially dangerous regex pattern: ${pattern}`); + continue; + } + // Create case-insensitive regex from the pattern const regex = new RegExp(pattern, 'gi'); let match: RegExpExecArray | null; + let iterations = 0; + const MAX_ITERATIONS = 10000; while ((match = regex.exec(text)) !== null) { + if (++iterations > MAX_ITERATIONS) { + console.warn(`Pattern exceeded max iterations: ${pattern}`); + break; + } // Avoid infinite loops on zero-length matches if (match[0].length === 0) { regex.lastIndex++; continue; }Recommend using RE2 (npm package
re2) for robust ReDoS protection, as it uses a non-backtracking engine. Alternatively, combinesafe-regexvalidation with the iteration limit above, noting thatsafe-regexuses heuristics with known false negatives.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/security/llm-guard/index.ts` around lines 1631 - 1660, The banTopicsPatterns loop currently compiles and executes user regexes directly (see config.banTopicsPatterns iteration and the new RegExp(pattern, 'gi') usage), which risks ReDoS; replace direct RegExp usage with a safe engine (preferably use the re2 package to construct patterns via new RE2(pattern, 'gi')) or, if re2 cannot be added, validate patterns before use with a safe-regex check and enforce execution safeguards (e.g., run matches in a worker with a short timeout or limit iterations) and ensure the findings.push logic (type 'BANNED_TOPIC', value, start, end, confidence) only runs after successful safe matching; also catch and log invalid or unsafe patterns instead of silently skipping them so operators can remediate bad patterns.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f3360019-721a-4d95-8d1a-eb9cb0539323
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (98)
docs/docs.jsondocs/features.mddocs/security/llm-guard.mdpackage.jsonscripts/refresh-llm-guard-patterns.mjssrc/__tests__/main/app-lifecycle/window-manager.test.tssrc/__tests__/main/ipc/handlers/process.test.tssrc/__tests__/main/ipc/handlers/security.test.tssrc/__tests__/main/preload/security.test.tssrc/__tests__/main/process-manager/handlers/ExitHandler.test.tssrc/__tests__/main/process-manager/handlers/StdoutHandler.test.tssrc/__tests__/main/security/llm-guard.test.tssrc/__tests__/main/security/llm-guard/config-export.test.tssrc/__tests__/main/security/llm-guard/recommendations.test.tssrc/__tests__/main/security/recommendations.test.tssrc/__tests__/main/security/security-logger.test.tssrc/__tests__/renderer/components/FindingDetails.test.tssrc/__tests__/renderer/components/LlmGuardIndicator.test.tssrc/__tests__/renderer/components/RightPanel.test.tsxsrc/__tests__/renderer/components/SanitizedContentDiff.test.tssrc/__tests__/renderer/components/SanitizedContentDiff.test.tsxsrc/__tests__/renderer/components/ScanProgressIndicator.test.tssrc/__tests__/renderer/components/SecurityBadge.test.tssrc/__tests__/renderer/components/SecurityEventsPanel.test.tssrc/__tests__/renderer/hooks/useSecurityToasts.test.tssrc/__tests__/setup.tssrc/main/app-lifecycle/window-manager.tssrc/main/group-chat/group-chat-router.tssrc/main/index.tssrc/main/ipc/handlers/index.tssrc/main/ipc/handlers/process.tssrc/main/ipc/handlers/security.tssrc/main/ipc/handlers/spellcheck.tssrc/main/preload/index.tssrc/main/preload/process.tssrc/main/preload/security.tssrc/main/preload/spellcheck.tssrc/main/process-listeners/forwarding-listeners.tssrc/main/process-manager/handlers/ExitHandler.tssrc/main/process-manager/handlers/StderrHandler.tssrc/main/process-manager/handlers/StdoutHandler.tssrc/main/process-manager/index.tssrc/main/process-manager/spawners/ChildProcessSpawner.tssrc/main/process-manager/types.tssrc/main/security/llm-guard/code-scanner.tssrc/main/security/llm-guard/config-export.tssrc/main/security/llm-guard/custom-patterns.tssrc/main/security/llm-guard/generated-patterns.tssrc/main/security/llm-guard/index.tssrc/main/security/llm-guard/patterns-metadata.jsonsrc/main/security/llm-guard/recommendations.tssrc/main/security/llm-guard/types.tssrc/main/security/llm-guard/url-scanner.tssrc/main/security/llm-guard/vault.tssrc/main/security/security-logger.tssrc/renderer/App.tsxsrc/renderer/components/AppModals.tsxsrc/renderer/components/FindingDetails.tsxsrc/renderer/components/InputArea.tsxsrc/renderer/components/LlmGuardIndicator.tsxsrc/renderer/components/MainPanel.tsxsrc/renderer/components/PromptComposerModal.tsxsrc/renderer/components/QuickActionsModal.tsxsrc/renderer/components/RightPanel.tsxsrc/renderer/components/SanitizedContentDiff.tsxsrc/renderer/components/ScanProgressIndicator.tsxsrc/renderer/components/SecurityBadge.tsxsrc/renderer/components/SecurityEventsPanel.tsxsrc/renderer/components/SecurityRecommendationsPanel.tsxsrc/renderer/components/SensitiveContentOverlay.tsxsrc/renderer/components/SensitiveContentPill.tsxsrc/renderer/components/SessionItem.tsxsrc/renderer/components/SessionList/SessionContextMenu.tsxsrc/renderer/components/SessionList/SessionList.tsxsrc/renderer/components/SessionSecurityModal.tsxsrc/renderer/components/Settings/SettingsModal.tsxsrc/renderer/components/Settings/tabs/LlmGuardTab.tsxsrc/renderer/components/Settings/tabs/index.tssrc/renderer/components/ToggleButtonGroup.tsxsrc/renderer/constants/modalPriorities.tssrc/renderer/constants/shortcuts.tssrc/renderer/global.d.tssrc/renderer/hooks/agent/useMergeTransferHandlers.tssrc/renderer/hooks/batch/useAutoRunHandlers.tssrc/renderer/hooks/index.tssrc/renderer/hooks/input/index.tssrc/renderer/hooks/input/useInputProcessing.tssrc/renderer/hooks/input/useSensitiveContentDetection.tssrc/renderer/hooks/keyboard/useMainKeyboardHandler.tssrc/renderer/hooks/remote/useRemoteHandlers.tssrc/renderer/hooks/security/index.tssrc/renderer/hooks/security/useSecurityToasts.tssrc/renderer/hooks/settings/useSettings.tssrc/renderer/stores/agentStore.tssrc/renderer/stores/modalStore.tssrc/renderer/stores/settingsStore.tssrc/renderer/types/index.tssrc/renderer/utils/sessionHelpers.ts
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/group-chat/group-chat-router.ts (1)
845-920:⚠️ Potential issue | 🔴 CriticalThe moderator sanitization can be bypassed through
historyContext.By the time this scan runs, the raw moderator message has already been appended to the chat log, and
historyContextis rebuilt from that log beforeparticipantPromptis created. Replacing only{{MESSAGE}}withsanitizedMessagestill leaves the unsanitized content in{{HISTORY_CONTEXT}}, so secrets/prompt-injection text can still reach participants. The scan needs to happen before logging/history generation, or the per-participant history needs to substitute the sanitized variant as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/group-chat/group-chat-router.ts` around lines 845 - 920, The moderator sanitization is ineffective because historyContext (used in participantPrompt) still contains the raw message; runLlmGuardInterAgent and use interAgentResult.sanitizedMessage to prevent bypass. Fix by moving the call to runLlmGuardInterAgent so it executes before the message is appended to the chat log/history, or when building participantPrompt (where groupChatParticipantRequestPrompt is populated) replace occurrences of the raw moderator message inside historyContext with interAgentResult.sanitizedMessage (or regenerate historyContext from the log with the sanitized entry) so that participantPrompt only ever includes sanitized content for each participant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/group-chat/group-chat-router.ts`:
- Around line 881-895: The loop that skips blocked participants (checking
interAgentResult.blocked) can leave the round stuck if every mentioned
participant is blocked because participantsToRespond becomes empty while the
mentions cleanup branch (mentions.length === 0) never runs; after processing
mentions, add a post-loop check: if the original mentions list existed
(mentions.length > 0) and participantsToRespond is now empty, explicitly reset
the round state — clear mentions/participantsToRespond, emit participant state
'idle' for any affected participant(s) via
groupChatEmitters.emitParticipantState, emit a system warning message via
groupChatEmitters.emitMessage indicating the round was cancelled, and release
any per-round power/block tokens so the power block leak is removed; reference
the interAgentResult.blocked check, participantsToRespond, mentions,
groupChatEmitters.emitParticipantState, groupChatEmitters.emitMessage,
participantName and groupChatId to locate where to add this post-loop reset
logic.
- Around line 846-852: The code is falling back to null LLM Guard settings by
calling getLlmGuardSettingsCallback?.() and allowing runLlmGuardInterAgent to
proceed unconfigured; change the logic in group-chat-router around the
getLlmGuardSettingsCallback invocation so routing is gated until the callback is
initialized (e.g., if getLlmGuardSettingsCallback is undefined or returns
undefined/null, return an error/hold the message rather than calling
runLlmGuardInterAgent), or alternatively ensure setGetLlmGuardSettingsCallback
is invoked before group-chat-router registers; apply the same guard pattern to
the other occurrence around runLlmGuardInterAgent at the 1128-1134 block so no
inter-agent scan runs with default/null config.
In `@src/main/process-manager/handlers/StdoutHandler.ts`:
- Around line 424-431: The current debug log in StdoutHandler logs raw
event.text and managedProcess.streamedText before sanitization; move or
duplicate the call to applyOutputGuard(sessionId, managedProcess, resultText) so
you compute guardedText first and then call logger.debug with guardedText (or
suppress previews) instead of logging event.text/managedProcess.streamedText,
and only after logging the sanitized preview call
this.bufferManager.emitDataBuffered(sessionId, guardedText); update references
in the block around applyOutputGuard, logger.debug, and
bufferManager.emitDataBuffered to ensure no raw synopsis is logged when LLM
Guard is enabled.
- Around line 530-590: The agent-error branches are currently emitting raw
agentError.message and raw.errorLine without LLM Guard; update those branches to
run the same guard flow (use runLlmGuardPost or call applyOutputGuard(sessionId,
managedProcess, messageText)) before emitting: pass the error string through
llmGuardState (managedProcess.llmGuardState) and handle the returned guardResult
exactly like applyOutputGuard does (log findings, create security events via
logSecurityEvent, emit 'security-event', and if blocked return the block message
or otherwise emit sanitizedResponse). Ensure you reference the same symbols used
in this file: applyOutputGuard, runLlmGuardPost, managedProcess.llmGuardState,
logSecurityEvent, and the emitter emit('security-event') so agent-error
emissions follow identical sanitization/blocking behavior.
In `@src/renderer/components/MainPanel.tsx`:
- Around line 435-438: Replace the direct selectors with defensive selectors:
update the useSettingsStore selectors for llmGuardEnabled and
llmGuardShowInputPreview in MainPanel (currently referenced as llmGuardEnabled
and llmGuardShowInputPreview) to use optional chaining and nullish coalescing
like the pattern in PromptComposerModal.tsx—i.e., read
s.llmGuardSettings?.enabled with a default of false, and read
s.llmGuardSettings?.showInputPreview with the appropriate default (true) so
missing/undefined llmGuardSettings won’t throw.
In `@src/renderer/components/RightPanel.tsx`:
- Around line 221-240: The refreshSecurityEvents() imperative handle currently
only triggers a refresh on the mounted SecurityEventsPanel and doesn't update
the parent RightPanel's securityEventIds state; modify refreshSecurityEvents to
also refresh the badge by invoking the same loader used in the useEffect
(loadEventIds) or by calling setSecurityEventIds with the latest IDs after
fetching (use window.maestro.security.getEvents), ensuring the parent state is
updated even when the Security tab is inactive so the unread badge stays
accurate; update any references to SecurityEventsPanel.refreshSecurityEvents to
rely on this combined behavior.
---
Outside diff comments:
In `@src/main/group-chat/group-chat-router.ts`:
- Around line 845-920: The moderator sanitization is ineffective because
historyContext (used in participantPrompt) still contains the raw message;
runLlmGuardInterAgent and use interAgentResult.sanitizedMessage to prevent
bypass. Fix by moving the call to runLlmGuardInterAgent so it executes before
the message is appended to the chat log/history, or when building
participantPrompt (where groupChatParticipantRequestPrompt is populated) replace
occurrences of the raw moderator message inside historyContext with
interAgentResult.sanitizedMessage (or regenerate historyContext from the log
with the sanitized entry) so that participantPrompt only ever includes sanitized
content for each participant.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a5501833-52c7-45aa-ac5f-86e0a667414b
📒 Files selected for processing (20)
docs/docs.jsondocs/features.mdpackage.jsonsrc/__tests__/main/ipc/handlers/system.test.tssrc/__tests__/main/process-manager/handlers/StdoutHandler.test.tssrc/__tests__/renderer/components/RightPanel.test.tsxsrc/main/group-chat/group-chat-router.tssrc/main/ipc/handlers/system.tssrc/main/process-manager/handlers/StdoutHandler.tssrc/renderer/components/MainPanel.tsxsrc/renderer/components/RightPanel.tsxsrc/renderer/components/SessionItem.tsxsrc/renderer/components/SessionList/SessionList.tsxsrc/renderer/global.d.tssrc/renderer/hooks/groupChat/useGroupChatHandlers.tssrc/renderer/hooks/input/useInputProcessing.tssrc/renderer/hooks/keyboard/useMainKeyboardHandler.tssrc/renderer/hooks/remote/useRemoteHandlers.tssrc/renderer/stores/agentStore.tssrc/renderer/types/index.ts
✅ Files skipped from review due to trivial changes (3)
- src/tests/main/ipc/handlers/system.test.ts
- docs/features.md
- src/main/ipc/handlers/system.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- src/tests/renderer/components/RightPanel.test.tsx
- src/tests/main/process-manager/handlers/StdoutHandler.test.ts
- package.json
- docs/docs.json
Comment on lines
+846
to
+852
| const llmGuardSettings = getLlmGuardSettingsCallback?.() ?? null; | ||
| const interAgentResult = runLlmGuardInterAgent( | ||
| message, | ||
| 'Moderator', | ||
| participantName, | ||
| llmGuardSettings | ||
| ); |
There was a problem hiding this comment.
Don't fail open when the LLM Guard callback is still unset.
Both paths silently fall back to null settings here. In this PR, src/main/index.ts wires setGetLlmGuardSettingsCallback(...) after group-chat registration, so early inter-agent traffic can bypass the stored policy and run with default/null config. Please gate routing until the callback is initialized, or move the callback registration earlier so scans never run unconfigured.
Also applies to: 1128-1134
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/group-chat/group-chat-router.ts` around lines 846 - 852, The code is
falling back to null LLM Guard settings by calling
getLlmGuardSettingsCallback?.() and allowing runLlmGuardInterAgent to proceed
unconfigured; change the logic in group-chat-router around the
getLlmGuardSettingsCallback invocation so routing is gated until the callback is
initialized (e.g., if getLlmGuardSettingsCallback is undefined or returns
undefined/null, return an error/hold the message rather than calling
runLlmGuardInterAgent), or alternatively ensure setGetLlmGuardSettingsCallback
is invoked before group-chat-router registers; apply the same guard pattern to
the other occurrence around runLlmGuardInterAgent at the 1128-1134 block so no
inter-agent scan runs with default/null config.
- Remove debug console.log statements from StdoutHandler.ts - Fix spellcheck locale validation to filter against availableSpellCheckerLanguages - Add inter_agent_scan to SecurityEventData and SecurityEvent types - Fix empty-string replacement handling in SanitizedContentDiff.tsx (use explicit undefined check) - Fix warn-only scans logged as input_scan (now properly uses 'warning' eventType) - Add .catch() handler to logSecurityEvent promise in process.ts - Forward promptLlmGuardEnabled prop to AppUtilityModals - Don't log raw spellcheck words (log length instead) - Fix forEach callback lint issues in recommendations.test.ts - Replace PAT-shaped fixtures with concatenated pieces in test files - Fix defensive selectors in MainPanel.tsx for llmGuardSettings Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The security-logger.ts module imports uuid (v4 as uuidv4) for generating unique security event IDs, but the package was not declared in dependencies. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
src/__tests__/main/process-manager/handlers/ExitHandler.test.ts (1)
284-287: Avoid hard-coding the exact redaction label in this handler test.Line 285 couples the test to secret-classifier internals (
GITHUB_TOKENnaming and_1numbering), so scanner tuning can break this without any regression inExitHandler. This test only needs to prove that the raw token was replaced by some redaction marker.♻️ Suggested assertion update
- expect(dataEvents[0]).toContain('john@acme.com'); - expect(dataEvents[0]).toContain('[REDACTED_SECRET_GITHUB_TOKEN_1]'); - expect(dataEvents[0]).not.toContain('[EMAIL_1]'); - expect(dataEvents[0]).not.toContain(githubToken); + const emitted = dataEvents.join(''); + expect(emitted).toContain('john@acme.com'); + expect(emitted).toMatch(/\[REDACTED_SECRET_[A-Z_]+_\d+\]/); + expect(emitted).not.toContain('[EMAIL_1]'); + expect(emitted).not.toContain(githubToken);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/process-manager/handlers/ExitHandler.test.ts` around lines 284 - 287, The test in ExitHandler.test.ts is asserting a specific redaction label ('[REDACTED_SECRET_GITHUB_TOKEN_1]') which couples the test to secret-classifier internals; instead, update the assertion that checks dataEvents[0] to verify a generic redaction marker exists and the raw token is removed: keep expect(dataEvents[0]).not.toContain(githubToken) and replace the exact-label assertion with a more robust check such as asserting the string contains 'REDACTED' or matches a regex like /\[REDACTED.*\]/ (using the existing dataEvents variable) so the test only ensures the token was redacted without depending on classifier naming/numbering.src/__tests__/main/security/llm-guard/recommendations.test.ts (1)
449-472: Consider adding explicit assertion that the recommendation exists before exclusion test.The test assumes
secret-detection-volumeis present inallbefore testing exclusion. If the recommendation isn't generated (e.g., threshold changes), thelength < all.lengthcomparison could pass vacuously.💡 Suggested improvement
const all = getRecommendations({ enabled: true }); + // Ensure the recommendation exists before testing exclusion + expect(all.find((r) => r.id === 'secret-detection-volume')).toBeDefined(); const withDismissed = getRecommendations( { enabled: true }, { excludeDismissed: true, dismissedIds: ['secret-detection-volume'] } ); expect(withDismissed.length).toBeLessThan(all.length);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/security/llm-guard/recommendations.test.ts` around lines 449 - 472, Ensure the test explicitly asserts that the 'secret-detection-volume' recommendation is present in the full set before testing exclusion: after calling getRecommendations({ enabled: true }) (variable all) add an assertion that all.find(r => r.id === 'secret-detection-volume') is defined (or expect(...).toBeDefined()) so the subsequent exclusion check using getRecommendations(..., { excludeDismissed: true, dismissedIds: ['secret-detection-volume'] }) (variable withDismissed) is meaningful; keep references to getRecommendations, the variables all and withDismissed, and the id 'secret-detection-volume' to locate where to insert this assertion.src/main/preload/security.ts (1)
15-31: Move these IPC contract types into a shared module.
SecurityEvent*, recommendation enums, andLlmGuardConfigForExportare now hand-copied in preload while the main-process handlers already have their own source types. This will drift quickly and stay “type-safe” on each side while breaking the IPC contract in practice. A single shared contract module would make schema changes much safer.Also applies to: 48-72, 126-243
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/preload/security.ts` around lines 15 - 31, Move the duplicated IPC contract types into a single shared module and have both preload and main-process import them: extract the interfaces and enums (e.g., SecurityEventData, any SecurityEvent* types, the recommendation enums, and LlmGuardConfigForExport) from the current preload copy into a new shared file, export them, then replace the hand-copied definitions in preload (and remove duplicates in the main-process) with imports from that shared module; update all references to those symbols (SecurityEventData, LlmGuardConfigForExport, the recommendation enums and other SecurityEvent* types) to use the shared exports and ensure the build and IPC handlers compile against the single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/app-lifecycle/window-manager.ts`:
- Around line 153-173: The current block only shows the "Add to Dictionary" item
when params.dictionarySuggestions has entries; update the logic so the whole
spelling menu is built when either params.dictionarySuggestions.length > 0 or
params.misspelledWord exists (e.g., compute hasSuggestions =
params.dictionarySuggestions?.length > 0 and hasMisspelling =
Boolean(params.misspelledWord)), push suggestion items only if hasSuggestions,
and then if hasMisspelling push a separator (only when menuItems not empty) and
push the "Add to Dictionary" item that calls
mainWindow.webContents.session.addWordToSpellCheckerDictionary(params.misspelledWord);
ensure you reference params.dictionarySuggestions, params.misspelledWord,
menuItems, and mainWindow.webContents.session.addWordToSpellCheckerDictionary
when making the change.
- Around line 127-148: Do not unconditionally call
webContents.session.setSpellCheckerLanguages() on every window creation; instead
check webContents.session.getSpellCheckerLanguages() and only seed defaults when
it returns an empty array, using app.getPreferredSystemLanguages() (or
app.getSystemLocale() fallback) to build candidates (with fallbacks like
language subtags and 'en-US') rather than app.getLocale(), and avoid running
this initialization on macOS where Session spell-check APIs are unsupported;
update the logic around setSpellCheckerLanguages / getSpellCheckerLanguages in
window-manager.ts to perform these conditional checks and use the
preferred-system-language sources so user-selected languages (and IPC changes
via spellcheck:setLanguages) are not overwritten.
In `@src/main/ipc/handlers/process.ts`:
- Around line 269-293: The preview and debug logging currently emit actual
prompt text via effectivePrompt and config.prompt (seen in the logFn prompt
preview block and logger.debug [RAW PROMPT] block); instead redact any
prompt-bearing fields before logging: remove substring previews and any direct
prompt/arg/fullCommand content and replace them with safe metadata (e.g.,
lengths, lengthDelta, containsHash boolean, containsNewline boolean, and a fixed
marker like "<REDACTED_PROMPT>"). Update the logFn call that uses
effectivePrompt and the logger.debug block that compares config.prompt vs
effectivePrompt to only emit numeric/boolean metadata and the
"<REDACTED_PROMPT>" placeholder, and apply the same change to the analogous
logging block referenced at lines 404-423; keep LOG_CONTEXT and sessionId
unchanged.
- Around line 224-232: The current call to logSecurityEvent is passing raw
guardResult.findings (which populates SecurityEvent.findings[].value). Instead,
transform guardResult.findings before calling logSecurityEvent: map each finding
to a safe representation that omits the original matched value and includes only
metadata such as type, start/end indexes (range), and a masked or redacted
snippet (e.g., fixed-length asterisks or token like "<REDACTED>"). Update the
payload passed to logSecurityEvent (the call in process.ts that currently uses
guardResult.findings) to use this mapped array while keeping other fields
(sessionId, tabId, eventType, action, originalLength, sanitizedLength)
unchanged.
In `@src/main/process-manager/handlers/StdoutHandler.ts`:
- Around line 327-330: The debug log currently records raw partial output
(event.text) into textPreview before applying the LLM guard, risking sensitive
data leakage; update the StdoutHandler flow so the preview uses the guarded
result from applyOutputGuard(sessionId, managedProcess, event.text) (the
guardedChunk used for emitter.emit('thinking-chunk', ...)) or suppress the
preview entirely when the guard is enabled, ensuring textPreview never stores
event.text unguarded.
- Around line 503-563: applyOutputGuard is currently stateless and is called per
stdout fragment, allowing secret/PII to slip through chunk boundaries and
allowing fragments after a block to still be emitted; change to maintain a
per-session rolling buffer (e.g., attach a guardBuffer string and
lastGuardedOffset to ManagedProcess or StdoutHandler keyed by sessionId) and
feed runLlmGuardPost the concatenation of guardBuffer + incoming fragment (or a
sliding-window overlap) instead of only the fragment; after scanning, update the
buffer to keep the last N characters (N >= max secret length or guard window),
emit only the sanitized portion that corresponds to the new fragment (or drop
emission if block detected) and ensure that if guardResult.blocked is true you
stop emitting any further fragments for that session (set a blocked flag on
managedProcess.llmGuardState) so later chunks are suppressed.
- Around line 534-555: The call to logSecurityEvent is currently fire-and-forget
and can produce unhandled promise rejections and lost realtime events; update
the code around logSecurityEvent(...) so you explicitly handle rejection (e.g.,
append .catch(err => { /* log via processLogger or this.logger with context:
sessionId, tabId, action */ })) and ensure the SecurityEventData is emitted to
this.emitter even if persistence fails (emit inside both the .then handler and
the .catch handler or emit once in a .finally block after constructing
eventData); reference the existing symbols logSecurityEvent, SecurityEventData,
this.emitter.emit('security-event', ...), sessionId and managedProcess.tabId
when adding the error handling and logging.
In `@src/renderer/components/SanitizedContentDiff.tsx`:
- Around line 718-731: The diff panes use hard-coded red/green styles instead of
the per-type colors from getHighlightColor; update the span styles in
originalSegments.map (removed) and the corresponding added segments and
inline-mode blocks to use the returned colors object (e.g., colors.background
for backgroundColor, colors.text for color and textDecorationColor) instead of
fixed rgba/ theme.colors.error or theme.colors.success values, and ensure the
title still includes segment.findingType; apply the same replacement wherever
getHighlightColor(...) is computed for the other diff/inline blocks.
- Around line 90-129: buildOriginalSegments assumes Finding.start and .end never
overlap; overlapping findings can cause duplicated/reordered text in the diff.
Before iterating, normalize the sorted findings (e.g., implement a
normalizeFindings function or inline logic) to merge or clip intersecting ranges
so that resulting findings are non-overlapping and in ascending order while
preserving replacements and types; then use the normalized list in
buildOriginalSegments (and apply the same normalization to the other builders
referenced in this file such as the corresponding sanitized-segments builder) to
ensure segments are produced deterministically and without duplicated/partial
spans.
- Around line 200-253: The inline builder currently ignores sanitizedContent and
always uses finding.replacement for added segments; update buildInlineSegments
to read added text from sanitizedContent instead by tracking a sanitizedIndex
pointer (e.g., let sanitizedPos = 0) alongside currentPos: for each
sortedFinding, push any normal/unchanged text by slicing
originalContent/currentPos and also slice sanitizedContent from sanitizedPos for
the corresponding unchanged/added spans, then for the added segment set text =
sanitizedContent.slice(sanitizedPos, sanitizedPos + (finding.replacement?.length
?? 0)) and advance sanitizedPos by that length (handle empty-string replacements
by advancing zero); finally, after the loop append any remaining
sanitizedContent.slice(sanitizedPos) as normal added/unchanged text so inline
mode uses the actual sanitizedContent instead of raw finding.replacement.
---
Nitpick comments:
In `@src/__tests__/main/process-manager/handlers/ExitHandler.test.ts`:
- Around line 284-287: The test in ExitHandler.test.ts is asserting a specific
redaction label ('[REDACTED_SECRET_GITHUB_TOKEN_1]') which couples the test to
secret-classifier internals; instead, update the assertion that checks
dataEvents[0] to verify a generic redaction marker exists and the raw token is
removed: keep expect(dataEvents[0]).not.toContain(githubToken) and replace the
exact-label assertion with a more robust check such as asserting the string
contains 'REDACTED' or matches a regex like /\[REDACTED.*\]/ (using the existing
dataEvents variable) so the test only ensures the token was redacted without
depending on classifier naming/numbering.
In `@src/__tests__/main/security/llm-guard/recommendations.test.ts`:
- Around line 449-472: Ensure the test explicitly asserts that the
'secret-detection-volume' recommendation is present in the full set before
testing exclusion: after calling getRecommendations({ enabled: true }) (variable
all) add an assertion that all.find(r => r.id === 'secret-detection-volume') is
defined (or expect(...).toBeDefined()) so the subsequent exclusion check using
getRecommendations(..., { excludeDismissed: true, dismissedIds:
['secret-detection-volume'] }) (variable withDismissed) is meaningful; keep
references to getRecommendations, the variables all and withDismissed, and the
id 'secret-detection-volume' to locate where to insert this assertion.
In `@src/main/preload/security.ts`:
- Around line 15-31: Move the duplicated IPC contract types into a single shared
module and have both preload and main-process import them: extract the
interfaces and enums (e.g., SecurityEventData, any SecurityEvent* types, the
recommendation enums, and LlmGuardConfigForExport) from the current preload copy
into a new shared file, export them, then replace the hand-copied definitions in
preload (and remove duplicates in the main-process) with imports from that
shared module; update all references to those symbols (SecurityEventData,
LlmGuardConfigForExport, the recommendation enums and other SecurityEvent*
types) to use the shared exports and ensure the build and IPC handlers compile
against the single source of truth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b5fbf50f-9077-461c-9198-1400508248e5
📒 Files selected for processing (12)
src/__tests__/main/process-manager/handlers/ExitHandler.test.tssrc/__tests__/main/process-manager/handlers/StdoutHandler.test.tssrc/__tests__/main/security/llm-guard/recommendations.test.tssrc/__tests__/renderer/components/SanitizedContentDiff.test.tsxsrc/main/app-lifecycle/window-manager.tssrc/main/ipc/handlers/process.tssrc/main/ipc/handlers/spellcheck.tssrc/main/preload/security.tssrc/main/process-manager/handlers/StdoutHandler.tssrc/renderer/components/AppModals.tsxsrc/renderer/components/MainPanel.tsxsrc/renderer/components/SanitizedContentDiff.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- src/renderer/components/AppModals.tsx
- src/main/ipc/handlers/spellcheck.ts
- src/tests/main/process-manager/handlers/StdoutHandler.test.ts
- src/renderer/components/MainPanel.tsx
Comment on lines
+127
to
+148
| // Configure spell-checker with system locale | ||
| // Uses system locale to respect user's language preference (en_US vs en_GB, etc.) | ||
| // Filter against availableSpellCheckerLanguages to avoid errors from unsupported locales | ||
| const systemLocale = app.getLocale(); | ||
| const available = new Set(mainWindow.webContents.session.availableSpellCheckerLanguages); | ||
| const candidates = [systemLocale, systemLocale.split('-')[0], 'en-US']; | ||
| const spellCheckLanguages = candidates.filter( | ||
| (lang, index) => !!lang && available.has(lang) && candidates.indexOf(lang) === index | ||
| ); | ||
|
|
||
| if (spellCheckLanguages.length > 0) { | ||
| mainWindow.webContents.session.setSpellCheckerLanguages(spellCheckLanguages); | ||
| logger.info( | ||
| `Spell-checker configured with languages: ${spellCheckLanguages.join(', ')}`, | ||
| 'Window' | ||
| ); | ||
| } else { | ||
| logger.warn( | ||
| `No supported spell-check dictionary found for locale ${systemLocale}`, | ||
| 'Window' | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the spellcheck handler
find . -path "*ipc/handlers/spellcheck*" -type fRepository: RunMaestro/Maestro
Length of output: 99
🏁 Script executed:
# Check the window-manager file for context around the spell-checker setup
cat -n src/main/app-lifecycle/window-manager.ts | sed -n '115,160p'Repository: RunMaestro/Maestro
Length of output: 2211
🌐 Web query:
Electron setSpellCheckerLanguages persistence getSpellCheckerLanguages API documentation
💡 Result:
In Electron, spellchecker language selection is part of the Session API:
ses.setSpellCheckerLanguages(languages: string[]): Enables spellcheck for the given language codes. Electron does not auto-detect the language you’re typing; you’re expected to set one or more languages explicitly. You can validate codes againstses.availableSpellCheckerLanguages(invalid codes error). [1]ses.getSpellCheckerLanguages(): string[]: Returns the currently-enabled language codes. If the list is empty, it falls back toen-US; on app launch, if it’s empty Electron will try to populate it from the current OS locale. This setting is persisted across restarts. [1]
Platform note: on macOS, Electron uses native spellchecking APIs and there’s no way to set the spellchecker language via Electron (the Session language APIs are for Windows/Linux). [2]
Sources: Electron Session API docs (methods + persistence note) [1]; Electron spellchecker tutorial (platform behavior) [2].
🏁 Script executed:
cat -n src/main/ipc/handlers/spellcheck.tsRepository: RunMaestro/Maestro
Length of output: 4758
🏁 Script executed:
# Also check what IPC handlers are registered and when
grep -n "spellcheck" src/main/index.tsRepository: RunMaestro/Maestro
Length of output: 44
🏁 Script executed:
grep -rn "registerSpellCheckHandlers" src/Repository: RunMaestro/Maestro
Length of output: 386
🏁 Script executed:
# Check src/main/index.ts to understand initialization flow
head -120 src/main/index.tsRepository: RunMaestro/Maestro
Length of output: 4275
🏁 Script executed:
# Check where registerSpellCheckHandlers is called in the IPC handlers index
sed -n '280,310p' src/main/ipc/handlers/index.tsRepository: RunMaestro/Maestro
Length of output: 665
🏁 Script executed:
# Check the createWindowManager and when windows are created
grep -n "createWindow" src/main/app-lifecycle/window-manager.ts | head -20Repository: RunMaestro/Maestro
Length of output: 438
🏁 Script executed:
# Check the main initialization flow in index.ts
sed -n '70,130p' src/main/index.tsRepository: RunMaestro/Maestro
Length of output: 2651
Reset spell-check languages unconditionally on every window creation, overwriting user preferences.
Line 138 calls setSpellCheckerLanguages() every time a window opens. Since Electron persists spell-check language selections across restarts and this PR exposes manual language changes via spellcheck:setLanguages IPC handler, user-selected languages get overwritten on the next launch. Additionally, app.getLocale() returns the application locale, not the system or user's language preference. Only seed spell-check languages when getSpellCheckerLanguages() returns an empty list, and derive the default from app.getPreferredSystemLanguages() or app.getSystemLocale() instead. Skip this initialization on macOS, where Electron's Session spell-check language APIs are not supported.
Suggested direction
- const systemLocale = app.getLocale();
- const available = new Set(mainWindow.webContents.session.availableSpellCheckerLanguages);
- const candidates = [systemLocale, systemLocale.split('-')[0], 'en-US'];
- const spellCheckLanguages = candidates.filter(
- (lang, index) => !!lang && available.has(lang) && candidates.indexOf(lang) === index
- );
-
- if (spellCheckLanguages.length > 0) {
- mainWindow.webContents.session.setSpellCheckerLanguages(spellCheckLanguages);
+ const ses = mainWindow.webContents.session;
+ const existingLanguages = ses.getSpellCheckerLanguages();
+
+ if (process.platform !== 'darwin' && existingLanguages.length === 0) {
+ const preferredLanguages = app.getPreferredSystemLanguages();
+ const available = new Set(ses.availableSpellCheckerLanguages);
+ const candidates = [
+ ...preferredLanguages.flatMap((lang) => [lang, lang.split('-')[0]]),
+ 'en-US',
+ ];
+ const spellCheckLanguages = candidates.filter(
+ (lang, index) => !!lang && available.has(lang) && candidates.indexOf(lang) === index
+ );
+
+ if (spellCheckLanguages.length > 0) {
+ ses.setSpellCheckerLanguages(spellCheckLanguages);
+ }
+ }
- logger.info(
- `Spell-checker configured with languages: ${spellCheckLanguages.join(', ')}`,
- 'Window'
- );
- } else {
+ if (existingLanguages.length === 0 && process.platform !== 'darwin') {
logger.warn(
- `No supported spell-check dictionary found for locale ${systemLocale}`,
+ 'No supported spell-check dictionary found for preferred system languages',
'Window'
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/app-lifecycle/window-manager.ts` around lines 127 - 148, Do not
unconditionally call webContents.session.setSpellCheckerLanguages() on every
window creation; instead check webContents.session.getSpellCheckerLanguages()
and only seed defaults when it returns an empty array, using
app.getPreferredSystemLanguages() (or app.getSystemLocale() fallback) to build
candidates (with fallbacks like language subtags and 'en-US') rather than
app.getLocale(), and avoid running this initialization on macOS where Session
spell-check APIs are unsupported; update the logic around
setSpellCheckerLanguages / getSpellCheckerLanguages in window-manager.ts to
perform these conditional checks and use the preferred-system-language sources
so user-selected languages (and IPC changes via spellcheck:setLanguages) are not
overwritten.
Comment on lines
+153
to
+173
| if (params.dictionarySuggestions && params.dictionarySuggestions.length > 0) { | ||
| const menuItems: Electron.MenuItemConstructorOptions[] = []; | ||
|
|
||
| // Add spelling suggestions | ||
| for (const suggestion of params.dictionarySuggestions) { | ||
| menuItems.push({ | ||
| label: suggestion, | ||
| click: () => { | ||
| mainWindow.webContents.replaceMisspelling(suggestion); | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| // Add separator and "Add to Dictionary" option | ||
| menuItems.push({ type: 'separator' }); | ||
| menuItems.push({ | ||
| label: 'Add to Dictionary', | ||
| click: () => { | ||
| mainWindow.webContents.session.addWordToSpellCheckerDictionary(params.misspelledWord); | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -p "window-manager.ts" src/Repository: RunMaestro/Maestro
Length of output: 102
🏁 Script executed:
cat -n src/main/app-lifecycle/window-manager.ts | sed -n '140,220p'Repository: RunMaestro/Maestro
Length of output: 3148
🏁 Script executed:
rg "params\.(misspelledWord|dictionarySuggestions|isEditable)" src/main/app-lifecycle/window-manager.ts -A 2 -B 2Repository: RunMaestro/Maestro
Length of output: 917
🏁 Script executed:
web_search
Electron spellchecker "Add to Dictionary" misspelledWord documentationRepository: RunMaestro/Maestro
Length of output: 159
🌐 Web query:
Electron spellchecker context menu misspelledWord parameter
💡 Result:
In Electron, misspelledWord is not a standalone parameter—it’s a field on the params object you receive from the webContents 'context-menu' event:
params.misspelledWord: string— the misspelled word under the cursor (if any). If there isn’t one, it will be an empty string/falsey. [1]params.dictionarySuggestions: string[]— replacement suggestions, and it’s only populated when there’s a misspelled word and spellcheck is enabled. [1]
Typical usage:
win.webContents.on('context-menu', (event, params) => {
for (const suggestion of params.dictionarySuggestions) {
// replace the misspelling
win.webContents.replaceMisspelling(suggestion);
}
if (params.misspelledWord) {
// add the misspelled word to the dictionary
win.webContents.session.addWordToSpellCheckerDictionary(params.misspelledWord);
}
});This example is shown in Electron’s spellchecker docs and relies on the same params.misspelledWord / params.dictionarySuggestions fields. [2]
Sources: [1] [2]
Show "Add to Dictionary" even when there are zero spelling suggestions.
Electron's spellchecker context menu should offer "Add to Dictionary" whenever params.misspelledWord exists, independent of whether params.dictionarySuggestions has suggestions. Currently, technical terms and names that produce zero suggestions fall through to the plain edit menu, preventing users from whitelisting them. The suggested fix correctly restructures the condition to if (hasSuggestions || hasMisspelling) and conditionally includes the separator and "Add to Dictionary" option only when needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/app-lifecycle/window-manager.ts` around lines 153 - 173, The current
block only shows the "Add to Dictionary" item when params.dictionarySuggestions
has entries; update the logic so the whole spelling menu is built when either
params.dictionarySuggestions.length > 0 or params.misspelledWord exists (e.g.,
compute hasSuggestions = params.dictionarySuggestions?.length > 0 and
hasMisspelling = Boolean(params.misspelledWord)), push suggestion items only if
hasSuggestions, and then if hasMisspelling push a separator (only when menuItems
not empty) and push the "Add to Dictionary" item that calls
mainWindow.webContents.session.addWordToSpellCheckerDictionary(params.misspelledWord);
ensure you reference params.dictionarySuggestions, params.misspelledWord,
menuItems, and mainWindow.webContents.session.addWordToSpellCheckerDictionary
when making the change.
Comment on lines
+224
to
+232
| void logSecurityEvent({ | ||
| sessionId: config.sessionId, | ||
| tabId: config.tabId, | ||
| eventType, | ||
| findings: guardResult.findings, | ||
| action, | ||
| originalLength: config.prompt!.length, | ||
| sanitizedLength: guardResult.sanitizedPrompt.length, | ||
| }) |
There was a problem hiding this comment.
Don't persist raw findings into the security event log.
Lines 224-232 pass guardResult.findings straight into logSecurityEvent, and the new preload contract exposes SecurityEvent.findings[].value for retrieval/export. That turns the audit log into a secondary store of the exact secrets/PII the guard detected. Persist masked snippets or type/range metadata instead of raw match values.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/ipc/handlers/process.ts` around lines 224 - 232, The current call to
logSecurityEvent is passing raw guardResult.findings (which populates
SecurityEvent.findings[].value). Instead, transform guardResult.findings before
calling logSecurityEvent: map each finding to a safe representation that omits
the original matched value and includes only metadata such as type, start/end
indexes (range), and a masked or redacted snippet (e.g., fixed-length asterisks
or token like "<REDACTED>"). Update the payload passed to logSecurityEvent (the
call in process.ts that currently uses guardResult.findings) to use this mapped
array while keeping other fields (sessionId, tabId, eventType, action,
originalLength, sanitizedLength) unchanged.
Comment on lines
+269
to
+293
| // Log prompt preview AFTER LLM Guard sanitization to avoid logging secrets | ||
| // On Windows, show prompt preview to help debug truncation issues | ||
| if (effectivePrompt && isWindows) { | ||
| logFn(`Prompt preview (post-sanitization)`, LOG_CONTEXT, { | ||
| sessionId: config.sessionId, | ||
| promptPreview: { | ||
| first50: effectivePrompt.substring(0, 50), | ||
| last50: effectivePrompt.substring(Math.max(0, effectivePrompt.length - 50)), | ||
| containsHash: effectivePrompt.includes('#'), | ||
| containsNewline: effectivePrompt.includes('\n'), | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| // Debug-level logging for raw prompt metadata when sanitization occurred | ||
| // Prefixed with [RAW PROMPT] to clearly indicate this is pre-sanitization metadata | ||
| // Only logs metadata (lengths, delta), never actual prompt content that may contain secrets | ||
| if (config.prompt && effectivePrompt && config.prompt !== effectivePrompt) { | ||
| logger.debug(`[RAW PROMPT] Pre-sanitization prompt metadata`, LOG_CONTEXT, { | ||
| sessionId: config.sessionId, | ||
| rawLength: config.prompt.length, | ||
| sanitizedLength: effectivePrompt.length, | ||
| lengthDelta: config.prompt.length - effectivePrompt.length, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Redact prompt content from logs, even after LLM Guard.
effectivePrompt is not guaranteed to be safe to log: in warn mode it can still be the original prompt, and even sanitized output can retain sensitive text. The preview log plus fullCommand, args, and prompt at info level will retain user content in logs, which undercuts the protection this feature is adding. Log lengths/counts only, or explicitly redact prompt-bearing fields before logging.
Also applies to: 404-423
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/ipc/handlers/process.ts` around lines 269 - 293, The preview and
debug logging currently emit actual prompt text via effectivePrompt and
config.prompt (seen in the logFn prompt preview block and logger.debug [RAW
PROMPT] block); instead redact any prompt-bearing fields before logging: remove
substring previews and any direct prompt/arg/fullCommand content and replace
them with safe metadata (e.g., lengths, lengthDelta, containsHash boolean,
containsNewline boolean, and a fixed marker like "<REDACTED_PROMPT>"). Update
the logFn call that uses effectivePrompt and the logger.debug block that
compares config.prompt vs effectivePrompt to only emit numeric/boolean metadata
and the "<REDACTED_PROMPT>" placeholder, and apply the same change to the
analogous logging block referenced at lines 404-423; keep LOG_CONTEXT and
sessionId unchanged.
Comment on lines
+503
to
+563
| private applyOutputGuard( | ||
| sessionId: string, | ||
| managedProcess: ManagedProcess, | ||
| resultText: string | ||
| ): string { | ||
| const guardState = managedProcess.llmGuardState; | ||
| if (!guardState?.config?.enabled) { | ||
| return resultText; | ||
| } | ||
|
|
||
| const guardResult = runLlmGuardPost(resultText, guardState.vault, guardState.config); | ||
| if (guardResult.findings.length > 0) { | ||
| logger.warn('[LLMGuard] Output findings detected', 'LLMGuard', { | ||
| sessionId, | ||
| toolType: managedProcess.toolType, | ||
| findings: guardResult.findings.map((finding) => finding.type), | ||
| }); | ||
| } | ||
|
|
||
| // Log and emit security event for output scan | ||
| if (guardResult.findings.length > 0 || guardResult.blocked || guardResult.warned) { | ||
| let action: SecurityEventAction = 'none'; | ||
| if (guardResult.blocked) { | ||
| action = 'blocked'; | ||
| } else if (guardResult.warned) { | ||
| action = 'warned'; | ||
| } else if (guardResult.findings.length > 0) { | ||
| action = 'sanitized'; | ||
| } | ||
|
|
||
| // Log to persistent security event store | ||
| logSecurityEvent({ | ||
| sessionId, | ||
| tabId: managedProcess.tabId, | ||
| eventType: guardResult.blocked ? 'blocked' : 'output_scan', | ||
| findings: guardResult.findings, | ||
| action, | ||
| originalLength: resultText.length, | ||
| sanitizedLength: guardResult.sanitizedResponse.length, | ||
| }).then((event) => { | ||
| // Emit to ProcessManager listeners for real-time UI forwarding | ||
| const eventData: SecurityEventData = { | ||
| sessionId: event.sessionId, | ||
| tabId: event.tabId, | ||
| eventType: event.eventType, | ||
| findingTypes: event.findings.map((f) => f.type), | ||
| findingCount: event.findings.length, | ||
| action: event.action, | ||
| originalLength: event.originalLength, | ||
| sanitizedLength: event.sanitizedLength, | ||
| }; | ||
| this.emitter.emit('security-event', eventData); | ||
| }); | ||
| } | ||
|
|
||
| if (guardResult.blocked) { | ||
| return `[Maestro LLM Guard blocked response] ${guardResult.blockReason ?? 'Sensitive content detected.'}`; | ||
| } | ||
|
|
||
| return guardResult.sanitizedResponse; | ||
| } |
There was a problem hiding this comment.
Chunk-by-chunk guarding still leaves a bypass.
applyOutputGuard() is stateless, but this file now calls it on arbitrary stdout fragments and partial text events. Any secret/PII pattern split across chunk boundaries can pass through unsanitized, and in block mode later chunks can still be emitted after the block placeholder. This needs a rolling overlap/buffer per session, or the scan has to run on the buffered stream instead of each fragment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/process-manager/handlers/StdoutHandler.ts` around lines 503 - 563,
applyOutputGuard is currently stateless and is called per stdout fragment,
allowing secret/PII to slip through chunk boundaries and allowing fragments
after a block to still be emitted; change to maintain a per-session rolling
buffer (e.g., attach a guardBuffer string and lastGuardedOffset to
ManagedProcess or StdoutHandler keyed by sessionId) and feed runLlmGuardPost the
concatenation of guardBuffer + incoming fragment (or a sliding-window overlap)
instead of only the fragment; after scanning, update the buffer to keep the last
N characters (N >= max secret length or guard window), emit only the sanitized
portion that corresponds to the new fragment (or drop emission if block
detected) and ensure that if guardResult.blocked is true you stop emitting any
further fragments for that session (set a blocked flag on
managedProcess.llmGuardState) so later chunks are suppressed.
Comment on lines
+534
to
+555
| logSecurityEvent({ | ||
| sessionId, | ||
| tabId: managedProcess.tabId, | ||
| eventType: guardResult.blocked ? 'blocked' : 'output_scan', | ||
| findings: guardResult.findings, | ||
| action, | ||
| originalLength: resultText.length, | ||
| sanitizedLength: guardResult.sanitizedResponse.length, | ||
| }).then((event) => { | ||
| // Emit to ProcessManager listeners for real-time UI forwarding | ||
| const eventData: SecurityEventData = { | ||
| sessionId: event.sessionId, | ||
| tabId: event.tabId, | ||
| eventType: event.eventType, | ||
| findingTypes: event.findings.map((f) => f.type), | ||
| findingCount: event.findings.length, | ||
| action: event.action, | ||
| originalLength: event.originalLength, | ||
| sanitizedLength: event.sanitizedLength, | ||
| }; | ||
| this.emitter.emit('security-event', eventData); | ||
| }); |
There was a problem hiding this comment.
Handle logSecurityEvent failures explicitly.
This fire-and-forget promise has no rejection path. If the persistent security logger fails, the main process gets an unhandled rejection and you silently lose the realtime event too.
Suggested fix
- logSecurityEvent({
+ void logSecurityEvent({
sessionId,
tabId: managedProcess.tabId,
eventType: guardResult.blocked ? 'blocked' : 'output_scan',
findings: guardResult.findings,
action,
originalLength: resultText.length,
sanitizedLength: guardResult.sanitizedResponse.length,
- }).then((event) => {
- // Emit to ProcessManager listeners for real-time UI forwarding
- const eventData: SecurityEventData = {
- sessionId: event.sessionId,
- tabId: event.tabId,
- eventType: event.eventType,
- findingTypes: event.findings.map((f) => f.type),
- findingCount: event.findings.length,
- action: event.action,
- originalLength: event.originalLength,
- sanitizedLength: event.sanitizedLength,
- };
- this.emitter.emit('security-event', eventData);
- });
+ })
+ .then((event) => {
+ // Emit to ProcessManager listeners for real-time UI forwarding
+ const eventData: SecurityEventData = {
+ sessionId: event.sessionId,
+ tabId: event.tabId,
+ eventType: event.eventType,
+ findingTypes: event.findings.map((f) => f.type),
+ findingCount: event.findings.length,
+ action: event.action,
+ originalLength: event.originalLength,
+ sanitizedLength: event.sanitizedLength,
+ };
+ this.emitter.emit('security-event', eventData);
+ })
+ .catch((error) => {
+ logger.warn('[LLMGuard] Failed to persist security event', 'LLMGuard', {
+ sessionId,
+ toolType: managedProcess.toolType,
+ error: error instanceof Error ? error.message : String(error),
+ });
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| logSecurityEvent({ | |
| sessionId, | |
| tabId: managedProcess.tabId, | |
| eventType: guardResult.blocked ? 'blocked' : 'output_scan', | |
| findings: guardResult.findings, | |
| action, | |
| originalLength: resultText.length, | |
| sanitizedLength: guardResult.sanitizedResponse.length, | |
| }).then((event) => { | |
| // Emit to ProcessManager listeners for real-time UI forwarding | |
| const eventData: SecurityEventData = { | |
| sessionId: event.sessionId, | |
| tabId: event.tabId, | |
| eventType: event.eventType, | |
| findingTypes: event.findings.map((f) => f.type), | |
| findingCount: event.findings.length, | |
| action: event.action, | |
| originalLength: event.originalLength, | |
| sanitizedLength: event.sanitizedLength, | |
| }; | |
| this.emitter.emit('security-event', eventData); | |
| }); | |
| void logSecurityEvent({ | |
| sessionId, | |
| tabId: managedProcess.tabId, | |
| eventType: guardResult.blocked ? 'blocked' : 'output_scan', | |
| findings: guardResult.findings, | |
| action, | |
| originalLength: resultText.length, | |
| sanitizedLength: guardResult.sanitizedResponse.length, | |
| }) | |
| .then((event) => { | |
| // Emit to ProcessManager listeners for real-time UI forwarding | |
| const eventData: SecurityEventData = { | |
| sessionId: event.sessionId, | |
| tabId: event.tabId, | |
| eventType: event.eventType, | |
| findingTypes: event.findings.map((f) => f.type), | |
| findingCount: event.findings.length, | |
| action: event.action, | |
| originalLength: event.originalLength, | |
| sanitizedLength: event.sanitizedLength, | |
| }; | |
| this.emitter.emit('security-event', eventData); | |
| }) | |
| .catch((error) => { | |
| logger.warn('[LLMGuard] Failed to persist security event', 'LLMGuard', { | |
| sessionId, | |
| toolType: managedProcess.toolType, | |
| error: error instanceof Error ? error.message : String(error), | |
| }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/process-manager/handlers/StdoutHandler.ts` around lines 534 - 555,
The call to logSecurityEvent is currently fire-and-forget and can produce
unhandled promise rejections and lost realtime events; update the code around
logSecurityEvent(...) so you explicitly handle rejection (e.g., append
.catch(err => { /* log via processLogger or this.logger with context: sessionId,
tabId, action */ })) and ensure the SecurityEventData is emitted to this.emitter
even if persistence fails (emit inside both the .then handler and the .catch
handler or emit once in a .finally block after constructing eventData);
reference the existing symbols logSecurityEvent, SecurityEventData,
this.emitter.emit('security-event', ...), sessionId and managedProcess.tabId
when adding the error handling and logging.
Comment on lines
+90
to
+129
| const buildOriginalSegments = (content: string, findings: Finding[]): TextSegment[] => { | ||
| const sortedFindings = sortFindingsByPosition(findings); | ||
| const segments: TextSegment[] = []; | ||
| let currentPos = 0; | ||
|
|
||
| for (const finding of sortedFindings) { | ||
| // Only process findings with replacements (i.e., actually sanitized) | ||
| // Use explicit undefined check to allow empty-string replacements (deletions) | ||
| if (finding.replacement === undefined) continue; | ||
|
|
||
| // Add normal text before this finding | ||
| if (finding.start > currentPos) { | ||
| segments.push({ | ||
| type: 'normal', | ||
| text: content.slice(currentPos, finding.start), | ||
| }); | ||
| } | ||
|
|
||
| // Add the removed (original) text | ||
| if (finding.start >= 0 && finding.end <= content.length) { | ||
| segments.push({ | ||
| type: 'removed', | ||
| text: content.slice(finding.start, finding.end), | ||
| findingType: finding.type, | ||
| replacement: finding.replacement, | ||
| }); | ||
| } | ||
|
|
||
| currentPos = finding.end; | ||
| } | ||
|
|
||
| // Add remaining normal text | ||
| if (currentPos < content.length) { | ||
| segments.push({ | ||
| type: 'normal', | ||
| text: content.slice(currentPos), | ||
| }); | ||
| } | ||
|
|
||
| return segments; |
There was a problem hiding this comment.
Normalize overlapping findings before building segments.
These builders assume Finding.start/end ranges never overlap. If two scanners flag intersecting spans, later iterations can slice from a position that is already consumed, which duplicates or reorders text in the rendered diff. In a security view, that can misrepresent what was sanitized or leave partial sensitive text visible.
Also applies to: 133-190, 200-253
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/SanitizedContentDiff.tsx` around lines 90 - 129,
buildOriginalSegments assumes Finding.start and .end never overlap; overlapping
findings can cause duplicated/reordered text in the diff. Before iterating,
normalize the sorted findings (e.g., implement a normalizeFindings function or
inline logic) to merge or clip intersecting ranges so that resulting findings
are non-overlapping and in ascending order while preserving replacements and
types; then use the normalized list in buildOriginalSegments (and apply the same
normalization to the other builders referenced in this file such as the
corresponding sanitized-segments builder) to ensure segments are produced
deterministically and without duplicated/partial spans.
Comment on lines
+200
to
+253
| const buildInlineSegments = ( | ||
| originalContent: string, | ||
| sanitizedContent: string, | ||
| findings: Finding[] | ||
| ): InlineSegment[] => { | ||
| const sortedFindings = sortFindingsByPosition(findings).filter( | ||
| (f) => f.replacement !== undefined | ||
| ); | ||
|
|
||
| if (sortedFindings.length === 0) { | ||
| return [{ type: 'normal', text: originalContent }]; | ||
| } | ||
|
|
||
| const segments: InlineSegment[] = []; | ||
| let currentPos = 0; | ||
|
|
||
| for (const finding of sortedFindings) { | ||
| // Use explicit undefined check to allow empty-string replacements (deletions) | ||
| if (finding.replacement === undefined) continue; | ||
|
|
||
| // Add normal text before this finding | ||
| if (finding.start > currentPos) { | ||
| segments.push({ | ||
| type: 'normal', | ||
| text: originalContent.slice(currentPos, finding.start), | ||
| }); | ||
| } | ||
|
|
||
| // Add the removed text (strikethrough) | ||
| segments.push({ | ||
| type: 'removed', | ||
| text: originalContent.slice(finding.start, finding.end), | ||
| findingType: finding.type, | ||
| }); | ||
|
|
||
| // Add the replacement text | ||
| segments.push({ | ||
| type: 'added', | ||
| text: finding.replacement, | ||
| findingType: finding.type, | ||
| }); | ||
|
|
||
| currentPos = finding.end; | ||
| } | ||
|
|
||
| // Add remaining normal text | ||
| if (currentPos < originalContent.length) { | ||
| segments.push({ | ||
| type: 'normal', | ||
| text: originalContent.slice(currentPos), | ||
| }); | ||
| } | ||
|
|
||
| return segments; |
There was a problem hiding this comment.
Use the actual sanitizedContent when building inline additions.
Line 202 takes sanitizedContent, but this function never reads it and always renders finding.replacement instead. Any post-processing beyond the raw finding replacement makes inline mode diverge from both the side-by-side view and the "Copy sanitized" output.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/SanitizedContentDiff.tsx` around lines 200 - 253, The
inline builder currently ignores sanitizedContent and always uses
finding.replacement for added segments; update buildInlineSegments to read added
text from sanitizedContent instead by tracking a sanitizedIndex pointer (e.g.,
let sanitizedPos = 0) alongside currentPos: for each sortedFinding, push any
normal/unchanged text by slicing originalContent/currentPos and also slice
sanitizedContent from sanitizedPos for the corresponding unchanged/added spans,
then for the added segment set text = sanitizedContent.slice(sanitizedPos,
sanitizedPos + (finding.replacement?.length ?? 0)) and advance sanitizedPos by
that length (handle empty-string replacements by advancing zero); finally, after
the loop append any remaining sanitizedContent.slice(sanitizedPos) as normal
added/unchanged text so inline mode uses the actual sanitizedContent instead of
raw finding.replacement.
Comment on lines
+718
to
+731
| {originalSegments.map((segment, idx) => { | ||
| if (segment.type === 'removed') { | ||
| const colors = getHighlightColor(segment.findingType || ''); | ||
| return ( | ||
| <span | ||
| key={idx} | ||
| className="px-0.5 rounded" | ||
| style={{ | ||
| backgroundColor: 'rgba(239, 68, 68, 0.2)', | ||
| textDecoration: 'line-through', | ||
| textDecorationColor: theme.colors.error, | ||
| color: theme.colors.error, | ||
| }} | ||
| title={`${segment.findingType}: ${segment.text} → ${segment.replacement}`} |
There was a problem hiding this comment.
Apply the per-type highlight colors in the diff panes too.
Line 720 and Line 786 compute getHighlightColor(), but the spans still use hard-coded red/green styling, and inline mode does the same. Right now every finding looks identical in the main diff, so the legend colors do not map back to the highlighted content.
Also applies to: 784-795, 851-880
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/SanitizedContentDiff.tsx` around lines 718 - 731, The
diff panes use hard-coded red/green styles instead of the per-type colors from
getHighlightColor; update the span styles in originalSegments.map (removed) and
the corresponding added segments and inline-mode blocks to use the returned
colors object (e.g., colors.background for backgroundColor, colors.text for
color and textDecorationColor) instead of fixed rgba/ theme.colors.error or
theme.colors.success values, and ensure the title still includes
segment.findingType; apply the same replacement wherever getHighlightColor(...)
is computed for the other diff/inline blocks.
StdoutHandler.ts: - Move synopsis logging after sanitization to avoid logging raw sensitive content - Use guardedText preview instead of raw event.text/streamedText RightPanel.tsx: - Extract loadSecurityEventIds as useCallback for reuse - Update refreshSecurityEvents to also refresh badge by calling loadSecurityEventIds group-chat-router.ts: - Add post-loop check when all mentioned participants are blocked - Reset state to idle and remove power block when no participants respond - Emit system warning message when round is cancelled due to all blocks - Fix historyContext sanitization bypass by rebuilding context per-participant - Use sanitized message in historyContext to prevent raw message leakage Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
src/main/process-manager/handlers/StdoutHandler.ts (3)
534-555:⚠️ Potential issue | 🟠 MajorHandle
logSecurityEvent()rejection and decouple UI emission from persistence.This promise chain still has no rejection path, so a storage failure can surface as an unhandled rejection and drop the realtime
security-eventemission entirely.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/process-manager/handlers/StdoutHandler.ts` around lines 534 - 555, The current promise chain calling logSecurityEvent(...) can reject and prevent or delay the realtime emitter; change the flow to emit the SecurityEventData to this.emitter immediately (build eventData from managedProcess/guardResult/resultText as now) and then call logSecurityEvent(...) separately and attach a .catch(...) to swallow/log persistence errors (or use an async IIFE with try/catch) so persistence failures don’t cause unhandled rejections or block emitter.emit; keep references to logSecurityEvent, SecurityEventData, this.emitter.emit, managedProcess, guardResult and resultText when making the change.
503-562:⚠️ Potential issue | 🟠 MajorStateless chunk scanning still leaves a split-token bypass.
applyOutputGuard()only sees the current fragment. Secrets or PII split across stdout boundaries can pass through unsanitized, and a blocked fragment does not stop later fragments from being emitted because no blocked state is retained.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/process-manager/handlers/StdoutHandler.ts` around lines 503 - 562, applyOutputGuard currently scans only the current fragment, allowing split-token bypasses and not preserving a blocked state; modify applyOutputGuard and use managedProcess.llmGuardState to keep a rolling buffer (e.g., tail of the last N chars/tokens) and a persistent blocked flag so each new fragment is prefixed with the saved tail before calling runLlmGuardPost, update llmGuardState with the new tail and blocked/warned status from guardResult, and if the state is blocked ensure subsequent calls immediately return the blocked message (or suppress output) instead of emitting later fragments; touch symbols: applyOutputGuard, managedProcess.llmGuardState, runLlmGuardPost, and where you currently log/emit security events so they reflect the cumulative scan results.
327-329:⚠️ Potential issue | 🟠 MajorGuard output before any preview logging.
applyOutputGuard()still runs after raw previews are written at Lines 254 and 317, so sensitive output can still reach main-process logs whenever LLM Guard is enabled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/process-manager/handlers/StdoutHandler.ts` around lines 327 - 329, The output guard is being applied after raw previews are emitted, so move the call to applyOutputGuard(sessionId, managedProcess, event.text) to execute before any preview logging/emitter emits; then pass the guarded result into all preview emit calls (e.g., replace usages that emit event.text with the guardedChunk), ensuring every preview event (such as emitter.emit('thinking-preview', ...) and emitter.emit('thinking-chunk', ...)) uses the guarded output; update all preview-emitting sites in StdoutHandler so no raw event.text is logged before applyOutputGuard runs.src/main/group-chat/group-chat-router.ts (1)
842-849:⚠️ Potential issue | 🟠 MajorDon't run inter-agent scans with a null settings callback.
getLlmGuardSettingsCallback?.() ?? nullstill fails open here.src/main/index.tswiressetGetLlmGuardSettingsCallback(...)after group-chat handlers are registered, so early moderator/participant traffic can bypass the stored policy and run with default or null guard settings.Also applies to: 1158-1164
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/group-chat/group-chat-router.ts` around lines 842 - 849, The code currently calls getLlmGuardSettingsCallback?.() ?? null and then runs runLlmGuardInterAgent even when settings are null, allowing early traffic to bypass stored policy; change the logic in group-chat-router.ts so you only call runLlmGuardInterAgent when getLlmGuardSettingsCallback is defined and returns a non-null value (i.e., if typeof getLlmGuardSettingsCallback === 'function' and the returned llmGuardSettings is truthy), otherwise skip the inter-agent scan or defer it; apply the same guard fix to the other occurrence around the runLlmGuardInterAgent call at the 1158-1164 region.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/group-chat/group-chat-router.ts`:
- Around line 842-849: Move the inter-agent scan earlier so raw message content
never reaches preview/logging code: call runLlmGuardInterAgent(message,
'Moderator', participantName, getLlmGuardSettingsCallback?.() ?? null) before
any code that constructs or logs raw message previews (the places that currently
render moderator/participant previews). Locate the runLlmGuardInterAgent
invocation and relocate it to immediately after the incoming message is
received, before any preview/preview-log generation; ensure the same change is
applied for the other occurrences (the sections handling forwarded messages) so
scanning always precedes preview creation.
In `@src/renderer/components/RightPanel.tsx`:
- Around line 136-137: The unread-state logic treats lastViewedSecurityEventId
(from useSettingsStore) as “never viewed” before settings finish loading,
causing badge flashes and potential overwrites when loadAllSettings() completes;
fix by gating any comparisons or writes to lastViewedSecurityEventId behind a
settings-hydrated flag (e.g., add or use an isHydrated/isLoaded boolean from
useSettingsStore or await loadAllSettings()), and only compute unread badges or
call setLastViewedSecurityEventId once hydration is complete and the real stored
value is available (also apply same gating to the related logic around the
Security tab open/newest ID update in RightPanel and the block that currently
updates lastViewedSecurityEventId).
- Around line 218-241: The loadSecurityEventIds callback can race such that
older getEvents() responses overwrite newer ones; make it last-write-wins by
adding a monotonically increasing request id stored in a ref (e.g.,
latestLoadIdRef) that loadSecurityEventIds increments before calling
window.maestro.security.getEvents, captures the current id locally, and only
calls setSecurityEventIds(result.events.map(...)) if the captured id equals
latestLoadIdRef.current; update any callers (mount, onSecurityEvent,
refreshSecurityEvents) unchanged so only the most recent response updates the
state.
---
Duplicate comments:
In `@src/main/group-chat/group-chat-router.ts`:
- Around line 842-849: The code currently calls getLlmGuardSettingsCallback?.()
?? null and then runs runLlmGuardInterAgent even when settings are null,
allowing early traffic to bypass stored policy; change the logic in
group-chat-router.ts so you only call runLlmGuardInterAgent when
getLlmGuardSettingsCallback is defined and returns a non-null value (i.e., if
typeof getLlmGuardSettingsCallback === 'function' and the returned
llmGuardSettings is truthy), otherwise skip the inter-agent scan or defer it;
apply the same guard fix to the other occurrence around the
runLlmGuardInterAgent call at the 1158-1164 region.
In `@src/main/process-manager/handlers/StdoutHandler.ts`:
- Around line 534-555: The current promise chain calling logSecurityEvent(...)
can reject and prevent or delay the realtime emitter; change the flow to emit
the SecurityEventData to this.emitter immediately (build eventData from
managedProcess/guardResult/resultText as now) and then call
logSecurityEvent(...) separately and attach a .catch(...) to swallow/log
persistence errors (or use an async IIFE with try/catch) so persistence failures
don’t cause unhandled rejections or block emitter.emit; keep references to
logSecurityEvent, SecurityEventData, this.emitter.emit, managedProcess,
guardResult and resultText when making the change.
- Around line 503-562: applyOutputGuard currently scans only the current
fragment, allowing split-token bypasses and not preserving a blocked state;
modify applyOutputGuard and use managedProcess.llmGuardState to keep a rolling
buffer (e.g., tail of the last N chars/tokens) and a persistent blocked flag so
each new fragment is prefixed with the saved tail before calling
runLlmGuardPost, update llmGuardState with the new tail and blocked/warned
status from guardResult, and if the state is blocked ensure subsequent calls
immediately return the blocked message (or suppress output) instead of emitting
later fragments; touch symbols: applyOutputGuard, managedProcess.llmGuardState,
runLlmGuardPost, and where you currently log/emit security events so they
reflect the cumulative scan results.
- Around line 327-329: The output guard is being applied after raw previews are
emitted, so move the call to applyOutputGuard(sessionId, managedProcess,
event.text) to execute before any preview logging/emitter emits; then pass the
guarded result into all preview emit calls (e.g., replace usages that emit
event.text with the guardedChunk), ensuring every preview event (such as
emitter.emit('thinking-preview', ...) and emitter.emit('thinking-chunk', ...))
uses the guarded output; update all preview-emitting sites in StdoutHandler so
no raw event.text is logged before applyOutputGuard runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ac26c4ad-22cc-4c5e-9c2c-47b10a1bc501
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
package.jsonsrc/main/group-chat/group-chat-router.tssrc/main/process-manager/handlers/StdoutHandler.tssrc/renderer/components/RightPanel.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
Comment on lines
+842
to
+849
| // Apply inter-agent security scanning (moderator -> participant) | ||
| const llmGuardSettings = getLlmGuardSettingsCallback?.() ?? null; | ||
| const interAgentResult = runLlmGuardInterAgent( | ||
| message, | ||
| 'Moderator', | ||
| participantName, | ||
| llmGuardSettings | ||
| ); |
There was a problem hiding this comment.
Move inter-agent scanning ahead of the raw message previews.
The scan happens after the raw message previews at Lines 644-645 and 1136-1138, so moderator and participant content can still land in process logs even when the forwarded message is sanitized or blocked.
Also applies to: 1157-1164
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/group-chat/group-chat-router.ts` around lines 842 - 849, Move the
inter-agent scan earlier so raw message content never reaches preview/logging
code: call runLlmGuardInterAgent(message, 'Moderator', participantName,
getLlmGuardSettingsCallback?.() ?? null) before any code that constructs or logs
raw message previews (the places that currently render moderator/participant
previews). Locate the runLlmGuardInterAgent invocation and relocate it to
immediately after the incoming message is received, before any
preview/preview-log generation; ensure the same change is applied for the other
occurrences (the sections handling forwarded messages) so scanning always
precedes preview creation.
Comment on lines
+136
to
+137
| const lastViewedSecurityEventId = useSettingsStore((s) => s.lastViewedSecurityEventId); | ||
| const setLastViewedSecurityEventId = useSettingsStore((s) => s.setLastViewedSecurityEventId); |
There was a problem hiding this comment.
Gate unread-state logic on settings hydration.
lastViewedSecurityEventId is restored asynchronously, so treating null as “never viewed” before hydration finishes can flash the badge to all-unread on startup. Worse, if the Security tab is already open, Line 263 can persist the newest ID and wipe the previously stored marker before loadAllSettings() finishes.
Suggested fix
+ const settingsLoaded = useSettingsStore((s) => s.settingsLoaded);
const lastViewedSecurityEventId = useSettingsStore((s) => s.lastViewedSecurityEventId);
const setLastViewedSecurityEventId = useSettingsStore((s) => s.setLastViewedSecurityEventId);
...
const unreadSecurityCount = useMemo(() => {
+ if (!settingsLoaded) {
+ return 0;
+ }
if (!lastViewedSecurityEventId || securityEventIds.length === 0) {
// If never viewed, all events are "new" but cap at 99
return securityEventIds.length > 0 ? Math.min(securityEventIds.length, 99) : 0;
}
...
- }, [securityEventIds, lastViewedSecurityEventId]);
+ }, [securityEventIds, lastViewedSecurityEventId, settingsLoaded]);
...
useEffect(() => {
- if (activeRightTab === 'security' && rightPanelOpen && securityEventIds.length > 0) {
+ if (
+ settingsLoaded &&
+ activeRightTab === 'security' &&
+ rightPanelOpen &&
+ securityEventIds.length > 0
+ ) {
// Mark the newest event as viewed
setLastViewedSecurityEventId(securityEventIds[0]);
}
- }, [activeRightTab, rightPanelOpen, securityEventIds, setLastViewedSecurityEventId]);
+ }, [
+ activeRightTab,
+ rightPanelOpen,
+ securityEventIds,
+ settingsLoaded,
+ setLastViewedSecurityEventId,
+ ]);Also applies to: 244-265
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/RightPanel.tsx` around lines 136 - 137, The
unread-state logic treats lastViewedSecurityEventId (from useSettingsStore) as
“never viewed” before settings finish loading, causing badge flashes and
potential overwrites when loadAllSettings() completes; fix by gating any
comparisons or writes to lastViewedSecurityEventId behind a settings-hydrated
flag (e.g., add or use an isHydrated/isLoaded boolean from useSettingsStore or
await loadAllSettings()), and only compute unread badges or call
setLastViewedSecurityEventId once hydration is complete and the real stored
value is available (also apply same gating to the related logic around the
Security tab open/newest ID update in RightPanel and the block that currently
updates lastViewedSecurityEventId).
Comment on lines
+218
to
+241
| // Security events for badge display | ||
| const [securityEventIds, setSecurityEventIds] = useState<string[]>([]); | ||
|
|
||
| // Load security event IDs - extracted as callback so it can be reused | ||
| const loadSecurityEventIds = useCallback(async () => { | ||
| try { | ||
| const result = await window.maestro.security.getEvents(100, 0); | ||
| setSecurityEventIds(result.events.map((e) => e.id)); | ||
| } catch { | ||
| // Ignore errors - security events are optional | ||
| } | ||
| }, []); | ||
|
|
||
| // Load security event IDs on mount and subscribe to updates | ||
| useEffect(() => { | ||
| loadSecurityEventIds(); | ||
|
|
||
| // Subscribe to real-time updates | ||
| const unsubscribe = window.maestro.security.onSecurityEvent(() => { | ||
| loadSecurityEventIds(); | ||
| }); | ||
|
|
||
| return unsubscribe; | ||
| }, [loadSecurityEventIds]); |
There was a problem hiding this comment.
Make loadSecurityEventIds() last-write-wins.
This callback is fired from mount, realtime updates, and refreshSecurityEvents(). Those requests can overlap, and an older getEvents() result can resolve last and roll the badge back to stale IDs.
Suggested fix
// Security events for badge display
const [securityEventIds, setSecurityEventIds] = useState<string[]>([]);
+ const securityEventLoadSeq = useRef(0);
// Load security event IDs - extracted as callback so it can be reused
const loadSecurityEventIds = useCallback(async () => {
+ const loadSeq = ++securityEventLoadSeq.current;
try {
const result = await window.maestro.security.getEvents(100, 0);
+ if (loadSeq !== securityEventLoadSeq.current) {
+ return;
+ }
setSecurityEventIds(result.events.map((e) => e.id));
} catch {
// Ignore errors - security events are optional
}
}, []);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Security events for badge display | |
| const [securityEventIds, setSecurityEventIds] = useState<string[]>([]); | |
| // Load security event IDs - extracted as callback so it can be reused | |
| const loadSecurityEventIds = useCallback(async () => { | |
| try { | |
| const result = await window.maestro.security.getEvents(100, 0); | |
| setSecurityEventIds(result.events.map((e) => e.id)); | |
| } catch { | |
| // Ignore errors - security events are optional | |
| } | |
| }, []); | |
| // Load security event IDs on mount and subscribe to updates | |
| useEffect(() => { | |
| loadSecurityEventIds(); | |
| // Subscribe to real-time updates | |
| const unsubscribe = window.maestro.security.onSecurityEvent(() => { | |
| loadSecurityEventIds(); | |
| }); | |
| return unsubscribe; | |
| }, [loadSecurityEventIds]); | |
| // Security events for badge display | |
| const [securityEventIds, setSecurityEventIds] = useState<string[]>([]); | |
| const securityEventLoadSeq = useRef(0); | |
| // Load security event IDs - extracted as callback so it can be reused | |
| const loadSecurityEventIds = useCallback(async () => { | |
| const loadSeq = ++securityEventLoadSeq.current; | |
| try { | |
| const result = await window.maestro.security.getEvents(100, 0); | |
| if (loadSeq !== securityEventLoadSeq.current) { | |
| return; | |
| } | |
| setSecurityEventIds(result.events.map((e) => e.id)); | |
| } catch { | |
| // Ignore errors - security events are optional | |
| } | |
| }, []); | |
| // Load security event IDs on mount and subscribe to updates | |
| useEffect(() => { | |
| loadSecurityEventIds(); | |
| // Subscribe to real-time updates | |
| const unsubscribe = window.maestro.security.onSecurityEvent(() => { | |
| loadSecurityEventIds(); | |
| }); | |
| return unsubscribe; | |
| }, [loadSecurityEventIds]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/RightPanel.tsx` around lines 218 - 241, The
loadSecurityEventIds callback can race such that older getEvents() responses
overwrite newer ones; make it last-write-wins by adding a monotonically
increasing request id stored in a ref (e.g., latestLoadIdRef) that
loadSecurityEventIds increments before calling
window.maestro.security.getEvents, captures the current id locally, and only
calls setSecurityEventIds(result.events.map(...)) if the captured id equals
latestLoadIdRef.current; update any callers (mount, onSecurityEvent,
refreshSecurityEvents) unchanged so only the most recent response updates the
state.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/renderer/components/MainPanel.tsx (1)
435-438:⚠️ Potential issue | 🟠 MajorUse the persisted LLM Guard toggle here, not just the feature flag.
This selector ignores the new Settings-level enable/disable state, so the header indicator and input guard can stay active even after the user turns LLM Guard off. Derive
llmGuardEnabledfrom both feature availability andllmGuardSettings?.enabled, then baseshowInputPreviewon that same effective enabled state.Suggested fix
- const llmGuardEnabled = useSettingsStore((s) => s.encoreFeatures.llmGuard); - const llmGuardShowInputPreview = useSettingsStore( - (s) => s.encoreFeatures.llmGuard && s.llmGuardSettings?.showInputPreview !== false - ); + const llmGuardEnabled = useSettingsStore( + (s) => s.encoreFeatures.llmGuard && (s.llmGuardSettings?.enabled ?? false) + ); + const llmGuardShowInputPreview = useSettingsStore( + (s) => + s.encoreFeatures.llmGuard && + (s.llmGuardSettings?.enabled ?? false) && + s.llmGuardSettings?.showInputPreview !== false + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/MainPanel.tsx` around lines 435 - 438, The selector currently sets llmGuardEnabled from only the feature flag and llmGuardShowInputPreview separately, which ignores the persisted Settings-level toggle; update the selectors to compute an effective enabled flag by combining encoreFeatures.llmGuard AND llmGuardSettings?.enabled (e.g., let llmGuardEnabled = useSettingsStore(s => s.encoreFeatures.llmGuard && s.llmGuardSettings?.enabled !== false)), then derive llmGuardShowInputPreview from that same effective enabled state (and s.llmGuardSettings?.showInputPreview !== false) so both the header indicator and input preview respect the persisted toggle; locate and change the useSettingsStore selectors for llmGuardEnabled and llmGuardShowInputPreview in MainPanel.tsx accordingly.
🧹 Nitpick comments (3)
src/renderer/components/PromptComposerModal.tsx (3)
394-406: Potential text wrapping mismatch between textarea and overlay.The textarea doesn't explicitly set
whiteSpace, defaulting to browser behavior, while the overlay explicitly useswhiteSpace: 'pre-wrap'(line 425). This could cause highlighting misalignment if newlines or consecutive spaces are handled differently.Consider adding explicit
whiteSpace: 'pre-wrap'to the textarea style to ensure consistent text rendering:♻️ Proposed fix
className="absolute inset-0 bg-transparent resize-none outline-none scrollbar-thin p-4" style={{ color: theme.colors.textMain, zIndex: 1, caretColor: theme.colors.textMain, // Explicit font styling for exact overlay match fontFamily: 'system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif', fontSize: '1rem', lineHeight: 1.625, + whiteSpace: 'pre-wrap', wordBreak: 'break-word', overflowWrap: 'break-word', }}Also applies to: 418-429
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/PromptComposerModal.tsx` around lines 394 - 406, The textarea in PromptComposerModal (the absolute overlay textarea element defined near the className "absolute inset-0 bg-transparent resize-none outline-none scrollbar-thin p-4") lacks an explicit whiteSpace style while the overlay uses whiteSpace: 'pre-wrap', which can cause wrapping/highlight misalignment; update the textarea's inline style to include whiteSpace: 'pre-wrap' (alongside existing fontFamily, fontSize, lineHeight, etc.) so both elements render spaces/newlines identically and preserve highlighting alignment.
109-114: Consider surfacing scan errors to users.The hook returns
errorandisScanningstates that are currently unused. If the security scan fails silently, users may believe their input has been scanned when it hasn't. Consider either:
- Displaying a subtle error indicator when
erroris non-null- Showing a brief loading state via
isScanningfor feedbackThis is optional since the overlay serves as a passive enhancement rather than a critical security gate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/PromptComposerModal.tsx` around lines 109 - 114, Destructure and use the hook's error and isScanning values from useSensitiveContentDetection (alongside the existing findings) so the UI reflects scan status: capture { findings: sensitiveFindings, error: sensitiveError, isScanning } from useSensitiveContentDetection(value, { enabled: llmGuardEnabled && isOpen, debounceMs: 300, minLength: 3 }), then surface a subtle indicator in PromptComposerModal (near the existing preview/overlay) that shows a brief loading state when isScanning is true and an unobtrusive error badge or tooltip when sensitiveError is non-null so users know the scan failed or is in progress.
84-87: Clarify the intent of the OR logic forllmGuardEnabled.The comment states that props may be stale due to memo boundaries, suggesting the store should take precedence. However, the OR logic (
||) means either source can enable the guard. If the intent is to prefer the store value when available, consider whether this logic matches the expected behavior, especially if a parent explicitly passesllmGuardEnabled={false}but the store has it enabled.If the current behavior is intentional (either source can enable), the comment could be updated to clarify this.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/components/PromptComposerModal.tsx` around lines 84 - 87, The OR logic for llmGuardEnabled (computed from llmGuardEnabledFromStore and llmGuardEnabledProp via useSettingsStore) is ambiguous—decide whether the store should take precedence or either source may enable the guard; if store should win when present, replace the current || logic with a conditional that prefers llmGuardEnabledFromStore when it is explicitly defined (e.g., check typeof or nullish) and fall back to llmGuardEnabledProp, otherwise update the nearby comment to state that either source can enable the guard; reference llmGuardEnabledFromStore, llmGuardEnabledProp and llmGuardEnabled when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/MainPanel.tsx`:
- Around line 1431-1438: The LlmGuardIndicator is being passed the agent
conversation ID (activeTab?.agentSessionId) which is different from the Maestro
session ID used to scope security events; change the sessionId prop on
LlmGuardIndicator to use the Maestro session id (activeSession.id) instead so
events are filtered to the current session. Locate the LlmGuardIndicator usage
in MainPanel (check the JSX block gated by activeSession.inputMode === 'ai' and
llmGuardEnabled) and replace the sessionId value from activeTab?.agentSessionId
to activeSession.id.
---
Duplicate comments:
In `@src/renderer/components/MainPanel.tsx`:
- Around line 435-438: The selector currently sets llmGuardEnabled from only the
feature flag and llmGuardShowInputPreview separately, which ignores the
persisted Settings-level toggle; update the selectors to compute an effective
enabled flag by combining encoreFeatures.llmGuard AND llmGuardSettings?.enabled
(e.g., let llmGuardEnabled = useSettingsStore(s => s.encoreFeatures.llmGuard &&
s.llmGuardSettings?.enabled !== false)), then derive llmGuardShowInputPreview
from that same effective enabled state (and s.llmGuardSettings?.showInputPreview
!== false) so both the header indicator and input preview respect the persisted
toggle; locate and change the useSettingsStore selectors for llmGuardEnabled and
llmGuardShowInputPreview in MainPanel.tsx accordingly.
---
Nitpick comments:
In `@src/renderer/components/PromptComposerModal.tsx`:
- Around line 394-406: The textarea in PromptComposerModal (the absolute overlay
textarea element defined near the className "absolute inset-0 bg-transparent
resize-none outline-none scrollbar-thin p-4") lacks an explicit whiteSpace style
while the overlay uses whiteSpace: 'pre-wrap', which can cause
wrapping/highlight misalignment; update the textarea's inline style to include
whiteSpace: 'pre-wrap' (alongside existing fontFamily, fontSize, lineHeight,
etc.) so both elements render spaces/newlines identically and preserve
highlighting alignment.
- Around line 109-114: Destructure and use the hook's error and isScanning
values from useSensitiveContentDetection (alongside the existing findings) so
the UI reflects scan status: capture { findings: sensitiveFindings, error:
sensitiveError, isScanning } from useSensitiveContentDetection(value, { enabled:
llmGuardEnabled && isOpen, debounceMs: 300, minLength: 3 }), then surface a
subtle indicator in PromptComposerModal (near the existing preview/overlay) that
shows a brief loading state when isScanning is true and an unobtrusive error
badge or tooltip when sensitiveError is non-null so users know the scan failed
or is in progress.
- Around line 84-87: The OR logic for llmGuardEnabled (computed from
llmGuardEnabledFromStore and llmGuardEnabledProp via useSettingsStore) is
ambiguous—decide whether the store should take precedence or either source may
enable the guard; if store should win when present, replace the current || logic
with a conditional that prefers llmGuardEnabledFromStore when it is explicitly
defined (e.g., check typeof or nullish) and fall back to llmGuardEnabledProp,
otherwise update the nearby comment to state that either source can enable the
guard; reference llmGuardEnabledFromStore, llmGuardEnabledProp and
llmGuardEnabled when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 23c8020e-b00f-4268-a1d1-585f2f0ad0b7
📒 Files selected for processing (13)
src/main/index.tssrc/main/ipc/handlers/process.tssrc/renderer/components/MainPanel.tsxsrc/renderer/components/PromptComposerModal.tsxsrc/renderer/components/QuickActionsModal.tsxsrc/renderer/components/SessionList/SessionList.tsxsrc/renderer/components/Settings/SettingsModal.tsxsrc/renderer/components/Settings/tabs/EncoreTab.tsxsrc/renderer/components/Settings/tabs/LlmGuardTab.tsxsrc/renderer/hooks/keyboard/useMainKeyboardHandler.tssrc/renderer/hooks/security/useSecurityToasts.tssrc/renderer/stores/settingsStore.tssrc/renderer/types/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/renderer/components/QuickActionsModal.tsx
Comment on lines
+1431
to
+1438
| {/* LLM Guard Status Indicator - only show in AI mode when guard is enabled */} | ||
| {activeSession.inputMode === 'ai' && llmGuardEnabled && ( | ||
| <LlmGuardIndicator | ||
| theme={theme} | ||
| enabled={llmGuardEnabled} | ||
| sessionId={activeTab?.agentSessionId ?? undefined} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== SecurityEventData definitions ==="
rg -n -C2 "interface SecurityEventData|type SecurityEventData|sessionId\??:" src
echo
echo "=== Security event emitters / loggers ==="
rg -n -C3 "sessionId:|logSecurityEvent|onSecurityEvent|emit.*SecurityEvent" src/main src/renderer
echo
echo "=== Tab/session identifiers available near MainPanel ==="
rg -n -C3 "agentSessionId|activeTabId|interface .*Tab|type .*Tab" src/renderer src/sharedRepository: RunMaestro/Maestro
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== SecurityEventData interface definition ==="
find src -name "*.ts" -o -name "*.tsx" | xargs grep -l "SecurityEventData" | head -5
rg -n "export.*interface SecurityEventData|export.*type SecurityEventData" src
echo
echo "=== LlmGuardIndicator component ==="
fd "LlmGuardIndicator" src --type f
echo
echo "=== Security event emission - main process ==="
rg -n "sessionId.*Security|SecurityEvent.*sessionId|logSecurityEvent" src/main --type ts --type tsx -A 2 -B 2 | head -80Repository: RunMaestro/Maestro
Length of output: 729
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== SecurityEventData interface in types.ts ==="
sed -n '110,130p' src/main/process-manager/types.ts
echo
echo "=== SecurityEventData interface in security.ts ==="
sed -n '1,50p' src/main/preload/security.ts
echo
echo "=== LlmGuardIndicator component ==="
head -100 src/renderer/components/LlmGuardIndicator.tsx
echo
echo "=== Search for security event emission with sessionId ==="
rg -n "new SecurityEvent|emit.*Security|logSecurity" src/main/process-manager --type ts -A 3 -B 1 | head -100Repository: RunMaestro/Maestro
Length of output: 7092
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== ManagedProcess and sessionId ==="
grep -n "sessionId" src/main/process-manager/types.ts | head -20
echo
echo "=== LogSecurityEvent implementation ==="
rg -n "export.*function logSecurityEvent|export.*const logSecurityEvent" src/main/security
echo
echo "=== MainPanel activeSession and activeTab structure ==="
sed -n '1,100p' src/renderer/components/MainPanel.tsx | grep -E "interface|type.*Session|type.*Tab|activeSession|activeTab" || true
echo
echo "=== Lines around 1431 in MainPanel to see available context ==="
sed -n '1420,1450p' src/renderer/components/MainPanel.tsxRepository: RunMaestro/Maestro
Length of output: 2393
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Session interface definition ==="
rg -n "^export interface Session|^export type Session" src/renderer --type ts -A 20 | head -60
echo
echo "=== How activeSession is obtained in MainPanel ==="
grep -n "activeSession" src/renderer/components/MainPanel.tsx | head -10
echo
echo "=== Check if Session has id field and how it relates to sessionId ==="
rg -n "interface.*Session\s*{" src/shared src/renderer --type ts -A 15 | head -100
echo
echo "=== LlmGuardIndicator usage context - find where activeSession comes from ==="
sed -n '1,50p' src/renderer/components/MainPanel.tsx | grep -E "Session|activeSession|props"Repository: RunMaestro/Maestro
Length of output: 5455
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Session interface definition ==="
rg -n "interface Session" src/renderer/types src/shared --type ts -A 30 | head -150
echo
echo "=== Check activeTab structure ==="
sed -n '485,500p' src/renderer/components/MainPanel.tsxRepository: RunMaestro/Maestro
Length of output: 8452
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== LogSecurityEvent implementation ==="
sed -n '111,150p' src/main/security/security-logger.ts
echo
echo "=== Where logSecurityEvent is called - StdoutHandler ==="
sed -n '530,550p' src/main/process-manager/handlers/StdoutHandler.ts
echo
echo "=== Where logSecurityEvent is called - ExitHandler ==="
sed -n '60,80p' src/main/process-manager/handlers/ExitHandler.ts
echo
echo "=== Check what sessionId variable is in these handlers ==="
sed -n '1,50p' src/main/process-manager/handlers/StdoutHandler.ts | grep -E "sessionId|^import|^export|function|const"
echo
echo "=== Check AITabData interface ==="
rg -n "interface AITabData|type AITabData" src/renderer src/shared --type ts -A 15Repository: RunMaestro/Maestro
Length of output: 3300
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== StdoutHandler function signature and sessionId source ==="
sed -n '40,150p' src/main/process-manager/handlers/StdoutHandler.ts | head -80
echo
echo "=== AITabData interface ==="
rg -n "interface AITabData" src/renderer src/shared --type ts -A 12
echo
echo "=== Check how tab information flows - AITab or AITabData ==="
rg -n "AITab" src/renderer/types src/shared --type ts -B 2 -A 8 | head -80Repository: RunMaestro/Maestro
Length of output: 2513
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Find AITabData or AITab interfaces ==="
find src -name "*.ts" -o -name "*.tsx" | xargs grep -l "AITab" | head -10
echo
echo "=== Search in renderer types ==="
grep -n "interface AI" src/renderer/types/index.ts -A 10
echo
echo "=== Check how sessionId is passed in StdoutHandler ==="
grep -n "export.*handle\|async handle\|handle.*data" src/main/process-manager/handlers/StdoutHandler.ts | head -5
echo
echo "=== Get the handle method in StdoutHandler ==="
sed -n '100,200p' src/main/process-manager/handlers/StdoutHandler.tsRepository: RunMaestro/Maestro
Length of output: 1441
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Full handle method signature in StdoutHandler ==="
sed -n '200,250p' src/main/process-manager/handlers/StdoutHandler.ts
echo
echo "=== Check ManagedProcess and how sessionId relates to tabs ==="
sed -n '10,70p' src/main/process-manager/types.ts
echo
echo "=== Check what 'tabId' and 'sessionId' mean in managedProcess ==="
grep -n "managedProcess.sessionId\|managedProcess.tabId" src/main/process-manager/handlers/StdoutHandler.tsRepository: RunMaestro/Maestro
Length of output: 3747
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Verify activeSession has id field and is available in MainPanel ==="
sed -n '1400,1450p' src/renderer/components/MainPanel.tsx | grep -E "activeSession|LlmGuardIndicator" -B 3 -A 3
echo
echo "=== Confirm Session interface has id ==="
sed -n '575,580p' src/renderer/types/index.ts
echo
echo "=== Check if agentSessionId is actually agent-specific or session-specific ==="
grep -n "agentSessionId" src/renderer/types/index.ts -B 5 -A 2Repository: RunMaestro/Maestro
Length of output: 2672
Pass the Maestro session ID, not the agent session ID, to filter security events correctly.
LlmGuardIndicator only scopes events when sessionId is present. Security events are logged with the Maestro session ID (from ManagedProcess.sessionId), but the code passes activeTab?.agentSessionId, which is the agent's internal conversation ID—a different value. When agentSessionId is null, this passes undefined, causing the indicator to show all security events from every session instead of filtering to the current session only.
Change the sessionId prop to activeSession.id:
<LlmGuardIndicator
theme={theme}
enabled={llmGuardEnabled}
- sessionId={activeTab?.agentSessionId ?? undefined}
+ sessionId={activeSession.id}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {/* LLM Guard Status Indicator - only show in AI mode when guard is enabled */} | |
| {activeSession.inputMode === 'ai' && llmGuardEnabled && ( | |
| <LlmGuardIndicator | |
| theme={theme} | |
| enabled={llmGuardEnabled} | |
| sessionId={activeTab?.agentSessionId ?? undefined} | |
| /> | |
| )} | |
| {/* LLM Guard Status Indicator - only show in AI mode when guard is enabled */} | |
| {activeSession.inputMode === 'ai' && llmGuardEnabled && ( | |
| <LlmGuardIndicator | |
| theme={theme} | |
| enabled={llmGuardEnabled} | |
| sessionId={activeSession.id} | |
| /> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/components/MainPanel.tsx` around lines 1431 - 1438, The
LlmGuardIndicator is being passed the agent conversation ID
(activeTab?.agentSessionId) which is different from the Maestro session ID used
to scope security events; change the sessionId prop on LlmGuardIndicator to use
the Maestro session id (activeSession.id) instead so events are filtered to the
current session. Locate the LlmGuardIndicator usage in MainPanel (check the JSX
block gated by activeSession.inputMode === 'ai' and llmGuardEnabled) and replace
the sessionId value from activeTab?.agentSessionId to activeSession.id.
- Enhanced `docs.json` to include LLM Guard in the Encore Features section. - Updated `encore-features.md` to reflect the addition of LLM Guard with a description and quick start guide. - Improved `llm-guard.md` with detailed configuration options and detection types for better user guidance.
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
2 participants
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.
Summary
This PR introduces LLM Guard, a comprehensive security scanning pipeline that protects Maestro users from sensitive data exposure and malicious content when interacting with AI agents.
Core Security Features
User Interface
Settings & Configuration
Integration Points
useSensitiveContentDetectionhookuseSecurityToastshookAdditional Changes
Test Plan
Files Changed
Security Core (
src/main/security/llm-guard/)UI Components (
src/renderer/components/)Settings (
src/renderer/components/Settings/tabs/)Hooks (
src/renderer/hooks/)Tests (
src/__tests__/)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests