feat: permission improvements, correlation alerts, and audit logging#72
feat: permission improvements, correlation alerts, and audit logging#72TerrifiedBug merged 5 commits intomainfrom
Conversation
Backend changes: - Fix correlation alert entity_field bug in logs.py (was using sigma_field) - Add permission checks for viewer restrictions (comments, ownership) - Add PATCH endpoint for users to edit their own comments - Add logger import to alerts.py (was missing) - Grant analysts manage_settings permission for Index Patterns, Field Mappings, Health - Add updated_at field to alert comments model and schema - Create migration for alert_comments.updated_at column Frontend changes: - Add permission checks to bulk action buttons in Alerts page - Add edit/delete functionality for comments in AlertDetail - Hide add comment section for viewers (no manage_alerts permission) - Disable Take Ownership button for viewers - Show edit button only for own comments, delete only for admins - Add useAuth hook to Alerts page for permission checking
backend/alembic/versions/3726deb5f1be_add_updated_at_to_alert_comments.py
Fixed
Show fixed
Hide fixed
backend/alembic/versions/3726deb5f1be_add_updated_at_to_alert_comments.py
Fixed
Show fixed
Hide fixed
backend/alembic/versions/3726deb5f1be_add_updated_at_to_alert_comments.py
Fixed
Show fixed
Hide fixed
backend/alembic/versions/3726deb5f1be_add_updated_at_to_alert_comments.py
Fixed
Show fixed
Hide fixed
- Add MITRE tags from linked sigma rules to correlation alerts - Add links to source sigma rules in correlation alert UI - Fix "Assigned to me" pagination to show all assigned alerts - Create manage_index_config permission separate from manage_settings - Allow viewers to see Health page (read-only) - Restrict analyst Settings page access while keeping Index Patterns/Field Mappings - Fix admin delete comment button using isAdmin hook - Suppress ssl_context warning from opensearch-py
The Rule model doesn't have a tags attribute - tags are stored in the YAML content. Parse the yaml_content to extract tags from both linked sigma rules for correlation alerts.
| if parsed and isinstance(parsed, dict): | ||
| tags = parsed.get("tags", []) or [] | ||
| correlation_tags.extend(tags) | ||
| except (ValueError, yaml.YAMLError, Exception): |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
In general, empty except blocks should be replaced with handlers that either (1) perform some recovery/compensation, (2) re-raise or wrap the exception, or at minimum (3) log the exception with enough context to diagnose issues later. When failure should not abort the operation (as here, where tags are additive metadata), the best fix is to log the error and continue.
For this specific block, the ideal minimal-impact fix is:
- Keep the
try/exceptstructure and continue to swallow the error so alert creation is never blocked by a bad rule’s YAML. - Replace
passwith a log statement that records:- Which rule failed (
rule_a_id/rule_b_id). - That tags could not be loaded from YAML.
- The exception object.
- Which rule failed (
- Use the existing
loggeralready defined a few lines above in theif triggered_correlations:block, so no new imports or global loggers are required. - Apply the same change consistently to both the
rule_a_idandrule_b_idexceptblocks.
Concretely, within backend/app/api/logs.py, in the shown region:
- For the
except (ValueError, yaml.YAMLError, Exception):at line 417, replacepasswith alogger.warning(...)call mentioningrule_a_idand includingexc_info=Trueso stack traces are available if needed. - For the analogous
exceptat line 431, replacepasswith a similarlogger.warning(...)mentioningrule_b_id.
No new imports or helper methods are required; the logging import and logger variable are already in scope in this part of the function.
| @@ -414,8 +414,13 @@ | ||
| if parsed and isinstance(parsed, dict): | ||
| tags = parsed.get("tags", []) or [] | ||
| correlation_tags.extend(tags) | ||
| except (ValueError, yaml.YAMLError, Exception): | ||
| pass | ||
| except (ValueError, yaml.YAMLError, Exception) as e: | ||
| logger.warning( | ||
| "Failed to load tags from YAML for correlated rule_a_id %s: %s", | ||
| rule_a_id, | ||
| e, | ||
| exc_info=True, | ||
| ) | ||
|
|
||
| if rule_b_id: | ||
| try: | ||
| @@ -428,8 +433,13 @@ | ||
| if parsed and isinstance(parsed, dict): | ||
| tags = parsed.get("tags", []) or [] | ||
| correlation_tags.extend(tags) | ||
| except (ValueError, yaml.YAMLError, Exception): | ||
| pass | ||
| except (ValueError, yaml.YAMLError, Exception) as e: | ||
| logger.warning( | ||
| "Failed to load tags from YAML for correlated rule_b_id %s: %s", | ||
| rule_b_id, | ||
| e, | ||
| exc_info=True, | ||
| ) | ||
|
|
||
| # Deduplicate tags while preserving order | ||
| seen = set() |
| if parsed and isinstance(parsed, dict): | ||
| tags = parsed.get("tags", []) or [] | ||
| correlation_tags.extend(tags) | ||
| except (ValueError, yaml.YAMLError, Exception): |
Check notice
Code scanning / CodeQL
Empty except Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
General fix: empty except blocks should either (a) handle the error (e.g., log it, adjust control flow, or provide a fallback) or (b) be documented with a clear comment why ignoring is safe. Here, the appropriate fix is to treat YAML/DB issues as non-fatal but log them so operators can debug malformed rules.
Best minimal fix: keep the same behavior (alert creation continues even if tag parsing fails) but add logging in the except blocks for both rule_a and rule_b. The logging should include the rule ID and the exception details. We will not re‑raise; we’ll just record the problem. This matches the later pattern where WebSocket broadcast failures are logged but do not stop processing.
Concretely:
- In
backend/app/api/logs.py, within the shown code region:- Replace the
except (ValueError, yaml.YAMLError, Exception): passforrule_awith anexcept Exception as e:(or keep the tuple if desired) that logs a warning or error, then continues. - Apply the same change to the
rule_bblock at line 431.
- Replace the
- To do this we need a
loggeravailable in this module. The snippet already showslogger.error(...)later, so a logger is presumably defined earlier in the file; we will rely on that existing definition and just calllogger.warning(...)in the new code.
This adds observability without altering functional behavior: alerts are still created; tags enrichment remains best-effort, but failures are now visible.
| @@ -414,8 +414,13 @@ | ||
| if parsed and isinstance(parsed, dict): | ||
| tags = parsed.get("tags", []) or [] | ||
| correlation_tags.extend(tags) | ||
| except (ValueError, yaml.YAMLError, Exception): | ||
| pass | ||
| except (ValueError, yaml.YAMLError, Exception) as e: | ||
| # Best-effort enrichment: failure to parse tags should not block alert creation | ||
| logger.warning( | ||
| "Failed to load tags from correlation rule A '%s': %s", | ||
| rule_a_id, | ||
| e, | ||
| ) | ||
|
|
||
| if rule_b_id: | ||
| try: | ||
| @@ -428,8 +433,13 @@ | ||
| if parsed and isinstance(parsed, dict): | ||
| tags = parsed.get("tags", []) or [] | ||
| correlation_tags.extend(tags) | ||
| except (ValueError, yaml.YAMLError, Exception): | ||
| pass | ||
| except (ValueError, yaml.YAMLError, Exception) as e: | ||
| # Best-effort enrichment: failure to parse tags should not block alert creation | ||
| logger.warning( | ||
| "Failed to load tags from correlation rule B '%s': %s", | ||
| rule_b_id, | ||
| e, | ||
| ) | ||
|
|
||
| # Deduplicate tags while preserving order | ||
| seen = set() |
- Add correlation rule link to CorrelationAlertDetails component - Hide separate Rule card for correlation alerts - Shows correlation rule name if available, otherwise 'View Rule'
- Add audit logging for comment add/edit/delete operations - Add audit logging for alert assign/unassign operations - Exclude alembic/versions from CodeQL analysis (reduces FPs) New audit actions: - alert.comment_add - alert.comment_edit - alert.comment_delete - alert.assign - alert.unassign
Summary
manage_index_configfrommanage_settingsfor better access controlChanges
Permissions
manage_index_configpermission for index pattern managementmanage_settings, Field Mappings tomanage_index_configCorrelation Alerts
Audit Logging
alert.comment_add- Track comment additionsalert.comment_edit- Track comment editsalert.comment_delete- Track comment deletionsalert.assign- Track alert ownership assignmentsalert.unassign- Track alert ownership releasesBug Fixes
CodeQL
backend/alembic/versions/**from analysis (reduces FP noise)Test plan