Add compliance level filtering with validation and explicit allowlist access control for MCP servers, data sources, and LLM endpoints #30
Conversation
Co-authored-by: garland3 <1162675+garland3@users.noreply.github.com>
Co-authored-by: garland3 <1162675+garland3@users.noreply.github.com>
Co-authored-by: garland3 <1162675+garland3@users.noreply.github.com>
Co-authored-by: garland3 <1162675+garland3@users.noreply.github.com>
Co-authored-by: garland3 <1162675+garland3@users.noreply.github.com>
|
@copilot I feel concerned that the user can't see the select compliance level without opening the tools. Also the UI for rag doesn't seem to have a way to selct the complialcne level. If a │ |
…ctions - Show compliance level indicator in header with quick clear button - Add compliance filter to RAG panel matching tools panel - Auto-clear incompatible tool/prompt selections when compliance level changes - Prevent accidental use of non-compliant tools from previous sessions Co-authored-by: garland3 <1162675+garland3@users.noreply.github.com>
Co-authored-by: garland3 <1162675+garland3@users.noreply.github.com>
Excellent points! I've addressed all your concerns: 1. Compliance Level Visibility (commit 84f381d)
2. RAG Panel Compliance Filter (commit 84f381d)
3. Auto-Cleanup of Incompatible Selections (commit 84f381d)
4. Future Extensibility
The safety mechanism ensures users cannot accidentally mix data from different security environments, even if tools were selected in a previous session. |
|
@copilot make it so this compliance level setting can be transmitted to the backend to enable future backend changes that are dependent on compliance level. Also, add a app config feature flag tht can be set in the env vars (similar to the existing ones) to turn this capabililty on or off so I that I can do a slow role out. |
- Add FEATURE_COMPLIANCE_LEVELS_ENABLED flag to enable/disable compliance filtering - Transmit compliance_level_filter to backend in chat messages - Make all compliance UI components conditional on feature flag - Update .env.example with new feature flag Co-authored-by: garland3 <1162675+garland3@users.noreply.github.com>
Co-authored-by: garland3 <1162675+garland3@users.noreply.github.com>
Done! Implemented both features (commits 6b69750, 09f47ee): 1. Backend Transmission
2. Feature Flag for Rollout Control
When disabled, the feature is completely hidden and bypassed. When enabled, compliance filter is transmitted to backend with every chat message in the |
|
please @copilot add support for filtering the LLM endpoints as well. For example, external LLMs have one level of enterprise security settings, while internal LLMs can support internal IP information and have another. |
- Add compliance_level field to ModelConfig - Update llmconfig.yml files with compliance levels (External for external LLMs) - Modify /api/config to return models as objects with compliance_level - Update frontend to filter models by compliance level - Display compliance badges on model dropdown options - Handle backward compatibility for models as strings or objects Co-authored-by: garland3 <1162675+garland3@users.noreply.github.com>
Co-authored-by: garland3 <1162675+garland3@users.noreply.github.com>
|
@copilot ok. lets change the setup , so that in the json file it explicitly sets the allowed combinations. so each type has a sub key:value which defines a list of allowed other types in the session. I think this will make it clear if the administrator can clearly define the "allowed" types that can be loaded at the same time as the current defined type. |
- Replace hierarchical model with explicit allowlist in compliance-levels.json - Each compliance level now has "allowed_with" list defining compatible levels - Prevents dangerous combinations like HIPAA + Public (internet search with PII) - Update ComplianceLevelManager to use allowlist instead of hierarchy - Update frontend filtering to use allowlist - Update /api/compliance-levels endpoint to expose allowlist - Example: HIPAA allows [HIPAA, SOC2] but NOT Public or External - Tests pass (backend: 59/60, frontend: 15/15) Co-authored-by: garland3 <1162675+garland3@users.noreply.github.com>
Co-authored-by: garland3 <1162675+garland3@users.noreply.github.com>
Done! Changed to explicit allowlist model (commits 96bdc96, 2b0366c): Configuration ( {
"name": "HIPAA",
"allowed_with": ["HIPAA", "SOC2"]
}Security Fix:
Administrator Control:
Examples:
The hierarchical model has been completely replaced with explicit allowlists for maximum security and administrative control. |
|
@copilot please add at least 5 unit test for the frontend, and 10 for the backend to veryify the behavior and detect regressions in the future. |
Backend tests (19 new tests in test_compliance_manager.py): - Test configuration loading and validation - Test canonical name resolution and aliases - Test compliance level validation - Test allowlist-based access control - Test accessible levels calculation - Test permissive mode when no config exists Frontend tests (10 new tests in compliance.test.jsx): - Test allowlist-based access logic - Test HIPAA accessing only HIPAA and SOC2 - Test Public isolation (only Public resources) - Test FedRAMP allowlist (FedRAMP + SOC2) - Test None/unset permissive behavior - Test tool filtering by compliance level - Test security - HIPAA cannot access Public (prevents PII leakage) - Test allowlist is not bidirectional - Test prevention of mixing security environments Total: 29 new tests (19 backend + 10 frontend) All tests passing: Backend 23/23, Frontend 10/10 Co-authored-by: garland3 <1162675+garland3@users.noreply.github.com>
Done! Added comprehensive test coverage (commit 15c008e): Backend Tests (19 new tests):
Frontend Tests (10 new tests):
Total: 29 tests (19 backend + 10 frontend) Tests verify:
|
|
@copilot add a example json file in the config/defaults folder and override folder. |
- Add example compliance-levels.json to config/overrides folder - Move compliance level dropdown to Header (single source of truth) - Remove compliance dropdowns from ToolsPanel and RagPanel - Implement STRICT filtering: when compliance level selected, only resources with matching compliance_level show - Resources without compliance_level are hidden when filter is active (prevents Public MCPs showing in SOC2 sessions) - Update MarketplaceContext isComplianceAccessible to enforce strict mode - Update Header model filtering to use strict mode - Update frontend tests to reflect strict filtering behavior - Frontend build passes successfully Co-authored-by: garland3 <1162675+garland3@users.noreply.github.com>
Co-authored-by: garland3 <1162675+garland3@users.noreply.github.com>
Done! Implemented all three requests (commits 654e31b, 8ba68c1): 1. Example JSON Configuration
2. Centralized Compliance Dropdown in Header
3. Fixed Bug: Strict Filtering Mode
Technical Details:
All changes build successfully. 29 tests passing. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…l handling - Removed unused compliance level state and related logic from RagPanel and ToolsPanel components. - Cleaned up imports and ensured compliance levels are not processed unnecessarily. - Updated backend test script to correctly set the PYTHONPATH based on the execution context.
| await self.create_session(session_id, user_email) | ||
|
|
||
| logger.info(f"Reset session {session_id} for user {user_email}") | ||
| logger.info(f"Reset session {sanitize_for_logging(str(session_id))} for user {sanitize_for_logging(user_email)}") |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 27 days ago
To fully mitigate the risk of log confusion/injection, we should:
- Continue to sanitize user input to remove control characters (as already done).
- Additionally, clearly bracket or mark user inputs in the log entry so that even if the input is ambiguous (e.g., contains spaces or special characters), log parsers and reviewers can easily distinguish where user data ends/begins.
- This means, in any log entry containing user-provided data, delimit the input with a standard marker, such as wrapping it in square brackets, quotes, or similar.
- Apply this fix to the affected line (278) in
backend/application/chat/service.py. - No change to
sanitize_for_loggingis needed; it already boils down unwanted control characters. - Example log entry:
logger.info(f"Reset session {sid} for user [{sanitized_email}]")wheresanitized_emailissanitize_for_logging(user_email).
Changes to apply:
- In
backend/application/chat/service.py, update line 278 so the user email is bracketed/quoted inside the log entry to minimize ambiguity.
| @@ -275,7 +275,7 @@ | ||
| # Create a new session | ||
| await self.create_session(session_id, user_email) | ||
|
|
||
| logger.info(f"Reset session {sanitize_for_logging(str(session_id))} for user {sanitize_for_logging(user_email)}") | ||
| logger.info(f"Reset session {sanitize_for_logging(str(session_id))} for user [{sanitize_for_logging(user_email)}]") | ||
|
|
||
| return { | ||
| "type": "session_reset", |
|
|
||
| if not servers_with_discovery: | ||
| logger.info("No servers implement rag_discover_resources for user %s", username) | ||
| logger.info("No servers implement rag_discover_resources for user %s", sanitize_for_logging(username)) |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 27 days ago
To fully mitigate log injection vulnerabilities according to best practices, in addition to stripping control characters, user-supplied input should be wrapped in clear delimiters (e.g., double quotes), making it visually distinct in the log even if some printable (but odd) characters pass through. This prevents an attacker from crafting an input that could lead to ambiguous or misleading log entries, even if control characters are stripped.
The best way to implement this:
- Update the
sanitize_for_loggingfunction inbackend/core/utils.pyso that after control characters are stripped, the resulting string is wrapped in double quotes. - This ensures every log entry coming from untrusted user input is visually enclosed, making forgeries or confusing entries much harder or impossible.
Required changes:
- Edit only the implementation of
sanitize_for_logginginbackend/core/utils.py(lines 14–43). - No new imports are needed as all required functionality is already present.
| @@ -13,36 +13,37 @@ | ||
|
|
||
| def sanitize_for_logging(value: Any) -> str: | ||
| """ | ||
| Sanitize a value for safe logging by removing control characters. | ||
| Sanitize a value for safe logging by removing control characters and wrapping in quotes. | ||
|
|
||
| Removes ASCII control characters (C0 and C1 ranges) to prevent log injection | ||
| attacks and log corruption. This includes characters like newlines, tabs, | ||
| escape sequences, and other non-printable characters that could be used to | ||
| manipulate log output or inject fake log entries. | ||
| manipulate log output or inject fake log entries. Then wraps the result in double quotes | ||
| to further delimit user input in logs. | ||
|
|
||
| Args: | ||
| value: Any value to sanitize. If not a string, it will be converted | ||
| to string representation first. | ||
|
|
||
| Returns: | ||
| str: Sanitized string with all control characters removed. | ||
| str: Sanitized string with all control characters removed and wrapped in double quotes. | ||
|
|
||
| Examples: | ||
| >>> sanitize_for_logging("Hello\\nWorld") | ||
| 'HelloWorld' | ||
| '"HelloWorld"' | ||
| >>> sanitize_for_logging("Test\\x1b[31mRed\\x1b[0m") | ||
| 'TestRed' | ||
| '"TestRed"' | ||
| >>> sanitize_for_logging(123) | ||
| '123' | ||
| '"123"' | ||
| """ | ||
| if value is None: | ||
| return '' | ||
| return '""' | ||
| if not isinstance(value, str): | ||
| value = str(value) | ||
| return _CONTROL_CHARS_RE.sub('', value) | ||
| sanitized = _CONTROL_CHARS_RE.sub('', value) | ||
| return f'"{sanitized}"' | ||
|
|
||
|
|
||
|
|
||
| async def get_current_user(request: Request) -> str: | ||
| """Get current user from request state (set by middleware).""" | ||
| return getattr(request.state, 'user_email', 'test@test.com') |
| server, | ||
| username, | ||
| sanitize_for_logging(server), | ||
| sanitize_for_logging(username), |
Check failure
Code scanning / CodeQL
Log Injection High
Copilot Autofix
AI 27 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| } | ||
|
|
||
| logger.info(f"Got file stats for user {user_email}: {result}") | ||
| logger.info(f"Got file stats for user {sanitize_for_logging(user_email)}: {result}") |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 27 days ago
To fully mitigate log injection vulnerabilities, especially in the scenario where logs may be processed by Unicode-aware systems or rendered as HTML, update sanitize_for_logging in backend/core/utils.py to also remove or replace Unicode line/paragraph separator characters (U+2028 and U+2029). Additionally, ensure that when logging data structures (like result), you use structured logging (e.g., logger.info(msg, extra=dict)) or sanitize/encode all items that could potentially be user input, to prevent confusion or log spoofing. In this specific case, the key area to adjust is the implementation of sanitize_for_logging. Ensure it strips or safely replaces all characters that could cause log forgery (\r, \n, U+2028, U+2029).
The required change is to broaden the regex controlled by _CONTROL_CHARS_RE in backend/core/utils.py and explicitly remove those Unicode line/paragraph separators. No changes are needed in the logger call itself, as long as the sanitization is comprehensive and robust.
Changes required:
- In
backend/core/utils.py, update_CONTROL_CHARS_REto match also Unicode line/paragraph separators (\u2028,\u2029). - In
sanitize_for_logging, the string replacement or regular expression should strip those additional characters.
| @@ -9,11 +9,13 @@ | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| _CONTROL_CHARS_RE = re.compile(r'[\x00-\x1f\x7f-\x9f]') | ||
| # Also remove Unicode line/paragraph separators (U+2028/U+2029) in addition to control chars | ||
| _CONTROL_CHARS_RE = re.compile(r'[\x00-\x1f\x7f-\x9f\u2028\u2029]') | ||
|
|
||
| def sanitize_for_logging(value: Any) -> str: | ||
| """ | ||
| Sanitize a value for safe logging by removing control characters. | ||
| Sanitize a value for safe logging by removing control characters and | ||
| Unicode line/paragraph separators. | ||
|
|
||
| Removes ASCII control characters (C0 and C1 ranges) to prevent log injection | ||
| attacks and log corruption. This includes characters like newlines, tabs, | ||
| @@ -35,10 +34,10 @@ | ||
| >>> sanitize_for_logging(123) | ||
| '123' | ||
| """ | ||
| if value is None: | ||
| return '' | ||
| if not isinstance(value, str): | ||
| value = str(value) | ||
| # Remove all control chars, including Unicode line/paragraph separators | ||
| return _CONTROL_CHARS_RE.sub('', value) | ||
|
|
||
|
|
| f"User {sanitize_for_logging(current_user)} has access to {len(authorized_servers)} servers: {authorized_servers}\n" | ||
| f"Returning {len(tools_info)} server tool groups to frontend for user {sanitize_for_logging(current_user)}" |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 27 days ago
The core issue is that sanitize_for_logging only removes ASCII control characters ([\x00-\x1f\x7f-\x9f]), leaving a residual risk for Unicode line separators (e.g., \u2028, \u2029, etc.) or other rare non-ASCII log-breaking characters. To fix this, update the sanitize_for_logging function in backend/core/utils.py to also remove Unicode newline characters systematically. This will render logs resistant to log injection by both ASCII and Unicode-based newlines. Specifically:
- File to change:
backend/core/utils.py - Region to change: The implementation of
sanitize_for_logging.- Supplement
_CONTROL_CHARS_REor introduce a new regex to also remove\u2028and\u2029(and optionally, other Unicode whitespace or unusual line separators). - Ensure all control and line separator characters are removed before returning the sanitized value.
- No changes needed to the call sites, as this will harden the central sanitization utility.
- Supplement
No changes are needed in backend/routes/config_routes.py as long as all relevant sinks route through the improved sanitizer.
| @@ -10,28 +10,32 @@ | ||
| logger = logging.getLogger(__name__) | ||
|
|
||
| _CONTROL_CHARS_RE = re.compile(r'[\x00-\x1f\x7f-\x9f]') | ||
| # Also match Unicode line/paragraph separators commonly problematic for log injection | ||
| _UNICODE_LINE_SEPARATORS_RE = re.compile(u'[\u2028\u2029]') | ||
|
|
||
| def sanitize_for_logging(value: Any) -> str: | ||
| """ | ||
| Sanitize a value for safe logging by removing control characters. | ||
| Sanitize a value for safe logging by removing control characters and Unicode line separators. | ||
|
|
||
| Removes ASCII control characters (C0 and C1 ranges) to prevent log injection | ||
| attacks and log corruption. This includes characters like newlines, tabs, | ||
| escape sequences, and other non-printable characters that could be used to | ||
| manipulate log output or inject fake log entries. | ||
| Removes ASCII control characters (C0 and C1 ranges) and Unicode line/paragraph separators | ||
| to prevent log injection attacks and log corruption. This includes characters like newlines, | ||
| tabs, escape sequences, other non-printable characters, and unusual Unicode newlines that | ||
| could be used to manipulate log output or inject fake log entries. | ||
|
|
||
| Args: | ||
| value: Any value to sanitize. If not a string, it will be converted | ||
| to string representation first. | ||
|
|
||
| Returns: | ||
| str: Sanitized string with all control characters removed. | ||
| str: Sanitized string with all control and line separator characters removed. | ||
|
|
||
| Examples: | ||
| >>> sanitize_for_logging("Hello\\nWorld") | ||
| 'HelloWorld' | ||
| >>> sanitize_for_logging("Test\\x1b[31mRed\\x1b[0m") | ||
| 'TestRed' | ||
| >>> sanitize_for_logging("Bad\u2028Log") | ||
| 'BadLog' | ||
| >>> sanitize_for_logging(123) | ||
| '123' | ||
| """ | ||
| @@ -39,10 +27,12 @@ | ||
| return '' | ||
| if not isinstance(value, str): | ||
| value = str(value) | ||
| return _CONTROL_CHARS_RE.sub('', value) | ||
| # First remove ASCII control chars, then Unicode line/paragraph separators | ||
| value = _CONTROL_CHARS_RE.sub('', value) | ||
| value = _UNICODE_LINE_SEPARATORS_RE.sub('', value) | ||
| return value | ||
|
|
||
|
|
||
|
|
||
| async def get_current_user(request: Request) -> str: | ||
| """Get current user from request state (set by middleware).""" | ||
| return getattr(request.state, 'user_email', 'test@test.com') |
| json.dump(feedback_data, f, indent=2, ensure_ascii=False) | ||
|
|
||
| logger.info(f"Feedback submitted by {current_user}: rating={feedback.rating}, file={filename}") | ||
| logger.info(f"Feedback submitted by {sanitize_for_logging(current_user)}: rating={feedback.rating}, file={sanitize_for_logging(filename)}") |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 27 days ago
The fix is to ensure that all user-provided values interpolated into logged messages are properly sanitized to prevent log injection. For string-type user input (even those with tight validation or restricted domain), sanitize for log safety by removing \r and \n characters. For numeric values like rating, the current validation allows only integer values that do not contain line breaks, so further action is unnecessary, but for best practices, converting the value to string before sanitizing is prudent.
Specifically, in submit_feedback, on line 107, before logging, ensure rating is converted to a string and passed through sanitize_for_logging. The log call should be refactored so that all variables are sanitized in a uniform way: current_user, rating, and filename. This will entirely eliminate risk from any log interpolation, even if future fields are added. Do not sanitize the log message template text itself.
You should only change line(s) you are shown. In this case, only line 107 and nearby context should be modified.
| @@ -104,7 +104,11 @@ | ||
| with open(feedback_file, 'w', encoding='utf-8') as f: | ||
| json.dump(feedback_data, f, indent=2, ensure_ascii=False) | ||
|
|
||
| logger.info(f"Feedback submitted by {sanitize_for_logging(current_user)}: rating={feedback.rating}, file={sanitize_for_logging(filename)}") | ||
| logger.info( | ||
| f"Feedback submitted by {sanitize_for_logging(current_user)}: " | ||
| f"rating={sanitize_for_logging(str(feedback.rating))}, " | ||
| f"file={sanitize_for_logging(filename)}" | ||
| ) | ||
|
|
||
| return { | ||
| "message": "Feedback submitted successfully", |
| # Delete the file | ||
| feedback_file.unlink() | ||
| logger.info(f"Feedback {feedback_id} deleted by {admin_user}") | ||
| logger.info(f"Feedback {sanitize_for_logging(feedback_id)} deleted by {sanitize_for_logging(admin_user)}") |
Check failure
Code scanning / CodeQL
Log Injection High
Copilot Autofix
AI 27 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
…ers and improve logging safety
This commit introduces compliance level filtering for RAG servers during discovery. The changes include: - **Compliance Level Manager Path Fix:** Corrects the path calculation in `ComplianceLevelManager` to correctly locate the project root when searching for configuration files. - **RAG Server Filtering:** Updates `RAGMCPService.discover_data_sources` and `RAGMCPService.discover_servers` to accept an optional `user_compliance_level`. - **Compliance Check:** If a `user_compliance_level` is provided, RAG servers are filtered based on their configured `compliance_level` using the `ComplianceLevelManager.is_accessible` method. Servers that do not meet the user's compliance level are skipped. This ensures that users only discover and interact with RAG servers that are compliant with their access level.
Refactors the Copilot instructions document (`.github/copilot-instructions.md`) to be a concise AI Agent Guide for Atlas UI 3. The changes: - Renames the document title for clarity. - Focuses on essential setup steps using `uv` and the new `agent_start.sh` script. - Provides a high-level overview of the application architecture (backend/frontend components). - Summarizes configuration layering and key feature flags. - Briefly outlines MCP and RAG conventions. This update improves the clarity and efficiency of the guide for developers and AI agents interacting with the repository.
…mpliance levels - Add support for both camelCase and snake_case compliance levels in RAGMCPService - Extend resource parsing to handle RAG mock format with accessible_data_sources - Refactor RAGClient to return structured DataSource objects instead of strings for better type safety and compliance info - Update README with contributor guidance pointing to CLAUDE.md and copilot-instructions.md for up-to-date workflows This improves interoperability between MCP and RAG mock standards, enhances data handling, and provides clearer onboarding for contributors.
|
|
||
| if not authorized_servers: | ||
| logger.info("No authorized MCP servers for user %s", username) | ||
| logger.info("No authorized MCP servers for user %s", sanitize_for_logging(username)) |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 27 days ago
The best way to fix the problem is to make sanitize_for_logging even more robust by explicitly removing (or replacing) any instances of the newline (\n) and carriage return (\r) characters, in addition to the removal of control characters by regex. This should be implemented by adding .replace('\n', '').replace('\r', '') after applying the regex.
To do this, change the return statement of sanitize_for_logging in backend/core/utils.py (line 42) from:
return _CONTROL_CHARS_RE.sub('', value)to:
return _CONTROL_CHARS_RE.sub('', value).replace('\n', '').replace('\r', '')This change guarantees that no newline, carriage return, or similar log-breaking character can make it through, even if the regex fails (due to encoding, unicode, or implementation issues).
No imports or definition changes are required, just a single line edit.
| @@ -39,7 +39,7 @@ | ||
| return '' | ||
| if not isinstance(value, str): | ||
| value = str(value) | ||
| return _CONTROL_CHARS_RE.sub('', value) | ||
| return _CONTROL_CHARS_RE.sub('', value).replace('\n', '').replace('\r', '') | ||
|
|
||
|
|
||
|
|
| logger.info( | ||
| "Skipping RAG server %s due to compliance level mismatch (user: %s, server: %s)", | ||
| sanitize_for_logging(server), | ||
| sanitize_for_logging(user_compliance_level), |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 27 days ago
The best way to fix the problem is to make it even clearer in the log entry which substrings are user-controlled so that no ambiguity can arise. In addition to already stripping control characters via sanitize_for_logging, we should explicitly delimit or quote all user-supplied values included in logs. Wrapping user input in square brackets, angle brackets, or quotes (after sanitization) is a common practice to help ensure there's no confusion about where user input starts or ends. This change should be applied wherever user input is logged, specifically on line 79 of backend/domain/rag_mcp_service.py. No additional imports are necessary, since standard string formatting will suffice. Only direct user values need this handling; static or derived values do not.
| @@ -74,7 +74,7 @@ | ||
| filtered_servers.append(server) | ||
| else: | ||
| logger.info( | ||
| "Skipping RAG server %s due to compliance level mismatch (user: %s, server: %s)", | ||
| "Skipping RAG server %s due to compliance level mismatch (user: [%s], server: [%s])", | ||
| sanitize_for_logging(server), | ||
| sanitize_for_logging(user_compliance_level), | ||
| sanitize_for_logging(server_compliance_level), |
| ) | ||
| authorized_servers = filtered_servers | ||
| if not authorized_servers: | ||
| logger.info("No authorized MCP servers remain after compliance filtering for user %s", sanitize_for_logging(username)) |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 27 days ago
The single best way to fix the issue is to improve the sanitize_for_logging function to:
- Remove or encode all newline (
\n), carriage return (\r), and other ASCII control characters (which it already does). - Clearly bracket or quote user-provided input in the logs to avoid ambiguity and make it obvious where user input begins and ends.
- Escape percent signs (
%) so that user input cannot exploit format-string interpolation (although Python logging should not interpolate positional%-formatting when used with args, it's defense-in-depth). - (Optionally) Replace non-ASCII control characters, and truncate excessively long strings.
The fix should be made in the sanitize_for_logging definition in backend/core/utils.py. The improved function should:
- Remove all control characters (as before).
- Surround the sanitized value with e.g.
[...]or'...'in the log output. - Replace any
%with%%to guard against rare format-string attacks. - (Optional) Limit the length to, say, 256 chars to avoid massive log entries.
Update usages such as in backend/domain/rag_mcp_service.py so they get the new behavior automatically.
Required changes:
- Edit
backend/core/utils.pyat the definition ofsanitize_for_logging. - No new imports needed, as standard library suffices.
- No code changes are needed in
rag_mcp_service.pyas it reuses the sanitizer.
| @@ -13,33 +13,46 @@ | ||
|
|
||
| def sanitize_for_logging(value: Any) -> str: | ||
| """ | ||
| Sanitize a value for safe logging by removing control characters. | ||
| Sanitize a value for safe logging by removing control characters, escaping percent signs, | ||
| bracketing the value, and truncating to avoid log injection and confusion. | ||
|
|
||
| Removes ASCII control characters (C0 and C1 ranges) to prevent log injection | ||
| attacks and log corruption. This includes characters like newlines, tabs, | ||
| escape sequences, and other non-printable characters that could be used to | ||
| manipulate log output or inject fake log entries. | ||
| attacks and log corruption. Escapes percent signs (%) to break formatting attacks. | ||
| Brackets the value to make user-provided input visually clear and unambiguous. | ||
| Cuts long values at 256 characters. Returns '[empty]' for None inputs. | ||
|
|
||
| Args: | ||
| value: Any value to sanitize. If not a string, it will be converted | ||
| to string representation first. | ||
|
|
||
| Returns: | ||
| str: Sanitized string with all control characters removed. | ||
| str: Sanitized, bracketed, length-limited string with control and '%' | ||
| characters removed or escaped. | ||
|
|
||
| Examples: | ||
| >>> sanitize_for_logging("Hello\\nWorld") | ||
| 'HelloWorld' | ||
| '[HelloWorld]' | ||
| >>> sanitize_for_logging("Test\\x1b[31mRed\\x1b[0m") | ||
| 'TestRed' | ||
| '[TestRed]' | ||
| >>> sanitize_for_logging(123) | ||
| '123' | ||
| '[123]' | ||
| >>> sanitize_for_logging(None) | ||
| '[empty]' | ||
| >>> sanitize_for_logging("%(malicious)s") | ||
| '[%%(malicious)s]' | ||
| """ | ||
| if value is None: | ||
| return '' | ||
| return '[empty]' | ||
| if not isinstance(value, str): | ||
| value = str(value) | ||
| return _CONTROL_CHARS_RE.sub('', value) | ||
| # Remove control characters | ||
| value = _CONTROL_CHARS_RE.sub('', value) | ||
| # Escape percent signs to prevent format string misunderstandings | ||
| value = value.replace('%', '%%') | ||
| # Truncate to 256 chars | ||
| value = value[:256] | ||
| # Bracket the value to make clear it is user-provided | ||
| return f'[{value}]' | ||
|
|
||
|
|
||
|
|
| logger.info( | ||
| "Skipping RAG server %s due to compliance level mismatch (user: %s, server: %s)", | ||
| sanitize_for_logging(server), | ||
| sanitize_for_logging(user_compliance_level), |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 27 days ago
The best way to fix the problem is to clearly mark user-provided input in log entries, as recommended by secure coding guidelines. This can be done by wrapping user-supplied values in quotes and/or adding a prefix like [USER] to their value in the log message.
In practice:
- Update the log statement in
discover_servers(inbackend/domain/rag_mcp_service.pyat line 192) to wrap the output ofsanitize_for_logging(user_compliance_level)(and similar parameters) in single or double quotes (or with a[USER]prefix if clearer). - Optionally, update the
sanitize_for_loggingdocstring to explicitly recommend quoting in logs.
Only code in backend/domain/rag_mcp_service.py is affected for the direct log statement (the other files either only pass the value along, or are unrelated).
No new methods or imports are needed—just adjust the log call formatting.
| @@ -187,7 +187,7 @@ | ||
| filtered_servers.append(server) | ||
| else: | ||
| logger.info( | ||
| "Skipping RAG server %s due to compliance level mismatch (user: %s, server: %s)", | ||
| "Skipping RAG server %s due to compliance level mismatch (user: '%s', server: '%s')", | ||
| sanitize_for_logging(server), | ||
| sanitize_for_logging(user_compliance_level), | ||
| sanitize_for_logging(server_compliance_level), |
| "Skipping RAG resource %s:%s due to compliance level mismatch (user: %s, resource: %s)", | ||
| sanitize_for_logging(server), | ||
| sanitize_for_logging(rid), | ||
| sanitize_for_logging(user_compliance_level), |
Check failure
Code scanning / CodeQL
Log Injection High
Copilot Autofix
AI 27 days ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
| logger.warning(f"HTTP error discovering data sources for {user_name}: {exc.detail}") | ||
| # Return empty list for graceful degradation instead of raising | ||
| return [] | ||
| except Exception as exc: |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 27 days ago
To fix this vulnerability, before logging any message that incorporates user-controlled input (user_name), we should sanitize the value so it cannot introduce new lines or special characters that would disrupt log analysis or enable log forgery. The ideal solution is to strip (remove) any \n and \r characters from user_name. Because the project already imports sanitize_for_logging (from core.utils) in other modules (though not in this one), we should import it and use it for processing user-controlled input before interpolation. The affected region is line 118 in backend/modules/rag/client.py inside the RAGClient.discover_data_sources method. This function logs errors containing user_name (sourced from current_user), so the value should be sanitized before being used in the log statement.
Steps:
- In
backend/modules/rag/client.py, importsanitize_for_loggingfromcore.utils. - Before the log statement at line 118, sanitize
user_name(e.g. assign to a new variable). - Use the sanitized value in the log statement instead of the raw
user_name.
| @@ -6,8 +6,8 @@ | ||
| from pydantic import BaseModel | ||
|
|
||
| from core.http_client import create_rag_client | ||
| from core.utils import sanitize_for_logging | ||
|
|
||
|
|
||
| class DataSource(BaseModel): | ||
| """Represents a RAG data source with compliance information.""" | ||
| name: str | ||
| @@ -115,7 +114,8 @@ | ||
| data = await self.http_client.get(f"/v1/discover/datasources/{user_name}") | ||
| accessible_sources_data = data.get("accessible_data_sources", []) | ||
| except HTTPException as exc: | ||
| logger.warning(f"HTTP error discovering data sources for {user_name}: {exc.detail}") | ||
| safe_user_name = sanitize_for_logging(user_name) | ||
| logger.warning(f"HTTP error discovering data sources for {safe_user_name}: {exc.detail}") | ||
| # Return empty list for graceful degradation instead of raising | ||
| return [] | ||
| except Exception as exc: |
| logger.error(f"Unexpected error while discovering data sources for {user_name}: {exc}", exc_info=True) | ||
| return [] | ||
|
|
||
| # Parse the list of dictionaries into a list of DataSource objects |
Check failure
Code scanning / CodeQL
Log Injection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 27 days ago
To fix the log injection issue, the user-controlled input (user_name) must be sanitized before it is logged. This means removing problematic characters such as newlines (\n, \r), or marking the parameter clearly. The safest, and consistent with your imported helpers (see line 9: from core.utils import sanitize_for_logging in config_routes.py), is to introduce a simple sanitization utility which strips dangerous characters from logged user input. If a helper like sanitize_for_logging exists already, use it; otherwise, implement it in a safe way in the current file.
Specifically, change line 122 in backend/modules/rag/client.py, and any other similar log statements involving user_name (lines 95, 109, 118, 122). Sanitize user_name before logging:
- Add a helper function, if not present, to remove newlines (
str.replace('\n', '').replace('\r', '')) or import one if available. - Use this function to sanitize
user_namebefore it is interpolated into log messages.
What is needed:
- Ensure
sanitize_for_loggingis available (either defined locally, or imported fromcore.utils). - Before logging, call
sanitize_for_logging(user_name)and use the result in formatted messages.
| @@ -6,8 +6,8 @@ | ||
| from pydantic import BaseModel | ||
|
|
||
| from core.http_client import create_rag_client | ||
| from core.utils import sanitize_for_logging | ||
|
|
||
|
|
||
| class DataSource(BaseModel): | ||
| """Represents a RAG data source with compliance information.""" | ||
| name: str | ||
| @@ -90,9 +89,10 @@ | ||
| async def discover_data_sources(self, user_name: str) -> List[DataSource]: | ||
| """Discover data sources accessible by a user.""" | ||
| use_test_client = bool(self.mock_mode and self.test_client) | ||
| safe_user_name = sanitize_for_logging(user_name) | ||
| logger.info( | ||
| "discover_data_sources: user=%s strategy=%s mock_mode=%s test_client=%s", | ||
| user_name, | ||
| safe_user_name, | ||
| "TestClient" if use_test_client else "HTTP", | ||
| self.mock_mode, | ||
| self.test_client is not None, | ||
| @@ -106,7 +104,7 @@ | ||
| data = response.json() | ||
| accessible_sources_data = data.get("accessible_data_sources", []) | ||
| except Exception as exc: | ||
| logger.error(f"TestClient error while discovering data sources for {user_name}: {exc}", exc_info=True) | ||
| logger.error(f"TestClient error while discovering data sources for {safe_user_name}: {exc}", exc_info=True) | ||
| return [] | ||
|
|
||
| # HTTP mode using unified client | ||
| @@ -115,11 +113,11 @@ | ||
| data = await self.http_client.get(f"/v1/discover/datasources/{user_name}") | ||
| accessible_sources_data = data.get("accessible_data_sources", []) | ||
| except HTTPException as exc: | ||
| logger.warning(f"HTTP error discovering data sources for {user_name}: {exc.detail}") | ||
| logger.warning(f"HTTP error discovering data sources for {safe_user_name}: {exc.detail}") | ||
| # Return empty list for graceful degradation instead of raising | ||
| return [] | ||
| except Exception as exc: | ||
| logger.error(f"Unexpected error while discovering data sources for {user_name}: {exc}", exc_info=True) | ||
| logger.error(f"Unexpected error while discovering data sources for {safe_user_name}: {exc}", exc_info=True) | ||
| return [] | ||
|
|
||
| # Parse the list of dictionaries into a list of DataSource objects |
Implements compliance level tagging (e.g., SOC2, HIPAA, Public, External, Internal) for MCP servers, RAG data sources, and LLM endpoints with session-level filtering to prevent mixing data from different security environments. Includes standardized compliance definitions, validation, alias normalization, explicit allowlist access control, and strict filtering mode.
Backend Changes
backend/core/compliance.py): New module that manages compliance level definitions, validation, and allowlist-based access checkingconfig/defaults/compliance-levels.json): JSON file defining standard compliance levels with explicit allowlists defining which levels can be used togetherconfig/overrides/compliance-levels.json): Template file for administrators to customize compliance levels and allowlistscompliance_levelfield toMCPServerConfigandModelConfigFEATURE_COMPLIANCE_LEVELS_ENABLEDenvironment variable for controlled rollout (default:false)/api/configendpoint includescompliance_levelin tools, prompts, models, and RAG server responses/api/compliance-levelsendpoint exposes compliance level definitions and allowlists to frontenddiscover_servers()returnscomplianceLevelat both server and source levelscompliance_level_filterwith every chat message for future compliance-dependent featuresCompliance Level Definitions
Example
compliance-levels.json:{ "version": "2.0", "description": "Defines compliance level types and their allowed combinations", "levels": [ { "name": "Public", "description": "Publicly accessible data, no restrictions", "aliases": [], "allowed_with": ["Public"] }, { "name": "External", "description": "External services with basic enterprise security", "aliases": [], "allowed_with": ["External"] }, { "name": "Internal", "description": "Internal systems, can handle company IP information", "aliases": [], "allowed_with": ["Internal"] }, { "name": "SOC2", "description": "SOC 2 Type II compliant systems", "aliases": ["SOC-2", "SOC 2"], "allowed_with": ["SOC2"] }, { "name": "HIPAA", "description": "HIPAA compliant systems for healthcare data", "aliases": ["HIPAA-Compliant"], "allowed_with": ["HIPAA", "SOC2"] }, { "name": "FedRAMP", "description": "FedRAMP authorized systems for government data", "aliases": ["FedRAMP-Moderate", "FedRAMP-High"], "allowed_with": ["FedRAMP", "SOC2"] } ], "mode": "explicit_allowlist" }Validation & Normalization
WARNING: Invalid compliance level 'SOCII' for MCP server 'pdfbasic'. Valid levels: Public, External, Internal, SOC2, HIPAA, FedRAMP. Setting to None."SOC 2"→"SOC2","HIPAA-Compliant"→"HIPAA"None(not errors) for graceful degradationExplicit Allowlist Access Control with Strict Filtering
Each compliance level has an
allowed_witharray that explicitly defines which compliance levels can be used together in the same session:Strict Filtering Mode: When a compliance level is selected, only resources with matching compliance levels (per allowlist) are shown. Resources without a
compliance_levelfield are hidden when a filter is active. This prevents untagged/Public resources from appearing in secure sessions (e.g., prevents Public MCPs from showing when SOC2 is selected).This explicit allowlist model with strict filtering prevents dangerous resource combinations. For example, HIPAA sessions cannot access Public resources, preventing the combination of HIPAA patient records with public internet search tools where search terms containing PII could leak to public services.
Example Configurations
MCP configuration:
{ "pdfbasic": { "command": ["python", "mcp/pdfbasic/main.py"], "compliance_level": "SOC2" }, "calculator": { "command": ["python", "mcp/calculator/main.py"], "compliance_level": "Public" } }LLM configuration:
Frontend Changes
complianceLevelFilterto user selections (persisted in localStorage)compliance_level_filter/api/compliance-levelsendpointMarketplaceContextimplementsisComplianceAccessible()using explicit allowlist rules with strict filteringgetComplianceFilteredTools()andgetComplianceFilteredPrompts()use allowlist logic with strict modesetComplianceLevelFilterWithCleanupwrapperFeature Flag for Controlled Rollout
Set
FEATURE_COMPLIANCE_LEVELS_ENABLED=truein.envto enable the feature. When disabled (default), all compliance UI is hidden and filtering is bypassed.Backend Integration
The compliance level filter is transmitted to the backend with every chat message, enabling future compliance-dependent features:
Allowlist Filtering Use Cases
Scenario 1: Healthcare Data with HIPAA
Scenario 2: External vs Internal LLMs
compliance_level: "External"compliance_level: "Internal"Scenario 3: Public Data
Scenario 4: Government Data with FedRAMP
Administrator Control
Administrators have full control over allowed compliance level combinations by editing the
allowed_witharrays incompliance-levels.json(or the override file inconfig/overrides/compliance-levels.json). This provides clear, explicit definitions of safe resource combinations for their specific environment and security requirements.Testing
backend/tests/test_compliance_manager.py): Configuration loading, validation, canonical name resolution, alias normalization, allowlist-based access control, accessible levels calculation, permissive mode, security validation (HIPAA cannot access Public), and non-bidirectional allowlistsfrontend/src/test/compliance.test.jsx): Allowlist-based accessibility logic with strict filtering, tool/model filtering, Public isolation, HIPAA allowlist (HIPAA + SOC2), FedRAMP allowlist (FedRAMP + SOC2), strict mode behavior (untagged resources hidden when filter active), prevention of mixing security environments, and non-bidirectional access controlStrict Filtering vs Backward Compatibility
compliance_level(backward compatible)compliance_levelare hidden (strict mode)This strict filtering ensures that when a user selects a compliance level (e.g., SOC2), they only see resources that are explicitly tagged with compatible compliance levels, preventing untagged/Public resources from appearing in secure sessions.
Security Benefits
Unified compliance system: Single source of truth for all compliance levels across MCP servers, RAG sources, and LLMs
Validated configurations: Invalid compliance levels are caught and warned about at config load time
Explicit allowlist control: Administrators explicitly define which compliance levels can be used together, preventing dangerous combinations
Strict filtering mode: Untagged resources hidden when compliance filter active, preventing accidental mixing
Prevents PII leakage: HIPAA sessions cannot access Public resources, preventing patient data exposure through internet search tools
Prevents data mixing: Users cannot accidentally mix data from different security environments
Centralized control: Single Header dropdown controls compliance for entire session (Tools, RAG, LLMs)
Always-visible indicator: Compliance level always shown in header so users know their current setting
Auto-cleanup: Prevents running non-compliant tools from previous sessions
Consistent filtering: Same strict allowlist logic across Tools, RAG Data Sources, and LLM model selection
Backend integration: Backend receives compliance context for future compliance-aware features
Extensible: Easy to add custom compliance levels and define their allowlists by updating
compliance-levels.jsonComprehensive test coverage: 29 tests ensure correct behavior and detect regressions
Fixes mcp and data source tags #17
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.