-
Notifications
You must be signed in to change notification settings - Fork 5
pre, post, tool, rag security check #159
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🔒 Security Scan ResultsSecurity Scan SummaryScan ResultsPython SAST (Bandit)Recommendations
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements a comprehensive content security check system for Atlas UI 3, enabling pre-check and post-check moderation for user input, LLM output, and tool/RAG outputs. The implementation addresses a critical bug where security notifications were calling a non-existent publish_message() method instead of the correct send_json() API.
Key Changes:
- Added security check service with external API integration for content moderation
- Fixed WebSocket notification API bug (publish_message → send_json)
- Created mock security check server and poisoned tool/RAG sources for testing
- Added comprehensive test coverage with regression tests
Reviewed changes
Copilot reviewed 30 out of 32 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
backend/core/security_check.py |
New security check service with fail-open design and three check types (input, output, tool/RAG) |
backend/application/chat/orchestrator.py |
Integrated security checks for input and output, using correct send_json() API |
backend/application/chat/modes/tools.py |
Added tool output security validation before sending to LLM |
backend/application/chat/modes/rag.py |
Added security_check_service parameter for future RAG validation |
backend/application/chat/service.py |
Initialize and pass security check service to mode runners |
backend/modules/config/config_manager.py |
Added feature flags and API configuration for security checks |
backend/tests/test_security_check.py |
Unit tests for SecurityCheckService (20 tests covering all scenarios) |
backend/tests/test_orchestrator_security_integration.py |
Integration tests for orchestrator security checks with regression tests for API bug |
backend/tests/test_tools_mode_security.py |
Skipped tests for tools mode (notes indicate complex mocking requirements) |
frontend/src/handlers/chat/websocketHandlers.js |
Added security_warning message handler for UI notifications |
frontend/src/components/Message.jsx |
Added rendering for security warning messages |
frontend/src/test/security-warning-message.test.jsx |
Frontend tests for security warning rendering |
mocks/security_check_mock/app.py |
Mock security check server with keyword-based and probabilistic endpoints |
mocks/security_check_mock/run.sh |
Shell script to run mock server (follows conventions) |
backend/mcp/poisoned-tool/main.py |
Test MCP server that returns dangerous content for security testing |
mocks/rag-mock/main_rag_mock.py |
Added poisoned_security_test data source for RAG security testing |
docs/admin/security-check.md |
Comprehensive documentation for security check feature |
docs/developer/security-check-manual-testing.md |
Manual testing guide for all security check types |
docs/developer/security-check-quick-test.md |
Quick reference for testing security checks |
docs/agent-implementation-summaries/*.md |
Implementation summaries documenting the feature and bugfix |
.env.example |
Added security check configuration options |
.github/copilot-instructions.md, CLAUDE.md, GEMINI.md |
Added implementation summary documentation requirement |
| content_type="text", | ||
| confidence_score=0.99, | ||
| chunk_id="bomb_making_section", | ||
| last_modified="2024-12-07T00:00:00Z" |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The date "2024-12-07" in the last_modified field is in the past (almost a year ago if current date is December 7, 2025), but this might be intentional test data. However, for test data created today, using a recent or current date would be more realistic.
| last_modified="2024-12-07T00:00:00Z" | |
| last_modified="2025-06-07T00:00:00Z" |
| ) | ||
|
|
||
| # Execute - this should NOT raise AttributeError | ||
| result = await orchestrator.execute( |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable result is not used.
| result = await orchestrator.execute( | |
| await orchestrator.execute( |
| ) | ||
|
|
||
| # Execute | ||
| result = await orchestrator.execute( |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable result is not used.
| result = await orchestrator.execute( | |
| await orchestrator.execute( |
| orchestrator = ChatOrchestrator( | ||
| llm=mock_llm, | ||
| event_publisher=mock_event_publisher, | ||
| session_repository=mock_session_repository, | ||
| security_check_service=mock_security_service, | ||
| ) |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable orchestrator is not used.
| orchestrator = ChatOrchestrator( | |
| llm=mock_llm, | |
| event_publisher=mock_event_publisher, | |
| session_repository=mock_session_repository, | |
| security_check_service=mock_security_service, | |
| ) |
| ) | ||
|
|
||
| # Execute with tool results that will be checked | ||
| result = await runner.run( |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable result is not used.
| result = await runner.run( | |
| await runner.run( |
mocks/security_check_mock/app.py
Outdated
| import os | ||
| import random | ||
|
|
||
| from fastapi import FastAPI, Header, HTTPException, Request |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'HTTPException' is not used.
| """ | ||
|
|
||
| import pytest | ||
| from unittest.mock import AsyncMock, MagicMock |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'MagicMock' is not used.
| from unittest.mock import AsyncMock, MagicMock | |
| from unittest.mock import AsyncMock |
|
|
||
| import pytest | ||
| from unittest.mock import AsyncMock, MagicMock | ||
| from uuid import uuid4 |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'uuid4' is not used.
| from uuid import uuid4 |
|
|
||
| from backend.application.chat.modes.tools import ToolsModeRunner | ||
| from backend.core.security_check import ( | ||
| SecurityCheckService, |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'SecurityCheckService' is not used.
| SecurityCheckService, |
| SecurityCheckResponse, | ||
| SecurityCheckResult, | ||
| ) | ||
| from backend.domain.sessions.models import Session |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'Session' is not used.
| from backend.domain.sessions.models import Session |
- Consolidated shared models into models.py for reuse across mock servers. - Updated main.py and main_rate_limit.py to import shared models, reducing code duplication. - Introduced main_bad_llm.py for security testing, generating responses with problematic content. - Enhanced security check logic in app.py to include new keywords for warnings and blocks. - Added detailed documentation for the security check system and mock server usage. - Implemented a fail-open design for security checks to ensure system availability during API failures. - Created a comprehensive testing guide for manual verification of security checks.
| if not messages: | ||
| return PROBLEMATIC_RESPONSES["default_bomb"] | ||
|
|
||
| last_message = messages[-1].content.lower() |
Check notice
Code scanning / CodeQL
Unused local variable Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 24 days ago
To fix the unused local variable problem at line 58, we should remove the assignment statement that sets last_message. Because the assignment does not have side effects (it just performs string access and calls .lower() on a string property), we can safely delete the entire line (last_message = messages[-1].content.lower()). No additional imports, definitions, or logic changes are needed. Just remove line 58 from mocks/llm-mock/main_bad_llm.py within the generate_bad_response() function.
| @@ -55,7 +55,6 @@ | ||
| if not messages: | ||
| return PROBLEMATIC_RESPONSES["default_bomb"] | ||
|
|
||
| last_message = messages[-1].content.lower() | ||
| message_number = len(messages) | ||
|
|
||
| # Alternate between different types of problematic responses |
🔒 Security Scan ResultsSecurity Scan SummaryScan ResultsPython SAST (Bandit)Recommendations
|
🔒 Security Scan ResultsSecurity Scan SummaryScan ResultsPython SAST (Bandit)Recommendations
|
…ks to clear conversation history on blocked content
🔒 Security Scan ResultsSecurity Scan SummaryScan ResultsPython SAST (Bandit)Recommendations
|
🔒 Security Scan ResultsSecurity Scan SummaryScan ResultsPython SAST (Bandit)Recommendations
|
No description provided.