Add security insights dashboard using existing issue data #5372 #5375
Conversation
|
👋 Hi @AyushPatiTripathi! This pull request needs a peer review before it can be merged. Please request a review from a team member who is not:
Once a valid peer review is submitted, this check will pass automatically. Thank you! |
WalkthroughAdds a new Django "dashboard" app, registers and routes it, and provides a login-protected Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📊 Monthly LeaderboardHi @AyushPatiTripathi! Here's how you rank for December 2025:
Leaderboard based on contributions in December 2025. Keep up the great work! 🚀 |
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| <script src="https://cdn.jsdelivr.net/npm/chart.js"></script> | ||
| <script> | ||
| /* DAILY TREND */ | ||
| new Chart(trendChart, { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
dashboard/views.py (1)
12-13: Add a docstring.The function should have a docstring explaining its purpose, parameters, and return value for better maintainability.
🔎 Proposed addition
@login_required def security_dashboard(request): + """ + Render the security operations dashboard showing security-related issues from the last 30 days. + + Displays KPIs, daily trends, severity distribution, status breakdown, and top affected domains + for issues with security label (label=4). + + Args: + request: HttpRequest object + + Returns: + HttpResponse with rendered dashboard template + """ today = timezone.now().date()website/templates/dashboard/security_dashboard.html (2)
38-38: Add accessibility features to charts.The canvas-based charts lack ARIA labels, alternative text, and keyboard navigation support, making the dashboard inaccessible to users with screen readers or other assistive technologies.
🔎 Proposed improvements
Add ARIA labels and role attributes to each canvas:
- <canvas id="trendChart"></canvas> + <canvas id="trendChart" role="img" aria-label="Line chart showing daily security issue trends over the last 30 days"></canvas>- <canvas id="statusChart"></canvas> + <canvas id="statusChart" role="img" aria-label="Doughnut chart showing issue status distribution"></canvas>- <canvas id="severityChart"></canvas> + <canvas id="severityChart" role="img" aria-label="Bar chart showing severity distribution based on CVSS scores"></canvas>- <canvas id="orgChart"></canvas> + <canvas id="orgChart" role="img" aria-label="Bar chart showing top 8 affected organizations"></canvas>Additionally, consider providing a data table below each chart as a fallback for users who cannot access the visual charts.
Also applies to: 44-44, 53-53, 59-59
182-241: Consider extracting inline styles to a separate CSS file.While inline styles work for a simple dashboard, extracting them to a dedicated CSS file would improve maintainability, enable reuse across other dashboard views, and allow for easier theming.
This is a low-priority enhancement that can be deferred to a future refactoring when the dashboard grows in complexity.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (7)
blt/settings.pyblt/urls.pydashboard/__init__.pydashboard/apps.pydashboard/urls.pydashboard/views.pywebsite/templates/dashboard/security_dashboard.html
🧰 Additional context used
🧬 Code graph analysis (2)
dashboard/views.py (1)
website/models.py (1)
Issue(590-739)
dashboard/urls.py (1)
dashboard/views.py (1)
security_dashboard(13-74)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run pre-commit
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (6)
blt/settings.py (1)
104-104: LGTM!The dashboard app is correctly registered in INSTALLED_APPS, and the app name matches the DashboardConfig defined in dashboard/apps.py.
blt/urls.py (1)
1254-1254: LGTM! Empty prefix pattern is acceptable.The empty prefix pattern correctly includes the dashboard URLs at the root level. Since dashboard/urls.py already defines the full path as "dashboard/security/", this results in the expected URL
/dashboard/security/.dashboard/apps.py (1)
1-6: LGTM!Standard Django app configuration is correctly implemented with appropriate default_auto_field and app name.
dashboard/urls.py (1)
1-7: LGTM!The URL configuration is clean and follows Django best practices. The path and name are descriptive and the view is correctly imported.
dashboard/views.py (1)
60-60: LGTM! Domain name handling is appropriate.The query correctly aggregates by domain name and limits to top 8. The template handles potential None values with the
|default:'Unknown'filter.website/templates/dashboard/security_dashboard.html (1)
95-97: LGTM! Use of|safefilter is appropriate here.The
daily_labelsanddaily_countsvariables are generated from date formatting and database aggregation (not user input), so disabling auto-escaping with the|safefilter is safe in this context.
| from django.contrib import admin | ||
| from django.contrib.auth.decorators import login_required | ||
| from django.urls import path, re_path | ||
| from django.urls import include, path, re_path |
There was a problem hiding this comment.
Remove duplicate import of include.
The include function is imported from both django.conf.urls (line 9) and django.urls (line 13). The import from django.conf.urls is deprecated in modern Django versions.
🔎 Proposed fix
Remove include from line 9 and keep only the import from django.urls:
-from django.conf.urls import include
from django.conf.urls.static import static📝 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.
| from django.urls import include, path, re_path | |
| from django.conf.urls.static import static | |
| from django.urls import include, path, re_path |
🤖 Prompt for AI Agents
In blt/urls.py around lines 9 and 13, there is a duplicate import of include
(once from django.conf.urls on line 9 and again from django.urls on line 13);
remove include from the django.conf.urls import on line 9 (keeping the import
from django.urls) so the deprecated import is eliminated and only the modern
django.urls.include is used, then run linters/tests to confirm no other
references require adjustment.
| </div> | ||
| </div> | ||
| <!-- CHART.JS --> | ||
| <script src="https://cdn.jsdelivr.net/npm/chart.js"></script> |
There was a problem hiding this comment.
External CDN dependency creates availability risk.
Loading Chart.js from a CDN introduces a dependency on external infrastructure. If the CDN becomes unavailable or is blocked by network policies, the dashboard will fail to render charts.
Consider one of these approaches:
- Bundle Chart.js with your static assets for better reliability
- Implement a CDN fallback mechanism
- Add a note in documentation about the CDN dependency
Example: CDN with local fallback
- <script src="https://cdn.jsdelivr.net/npm/chart.js"></script>
+ <script src="https://cdn.jsdelivr.net/npm/chart.js"></script>
+ <script>
+ // Fallback to local copy if CDN fails
+ if (typeof Chart === 'undefined') {
+ document.write('<script src="{% static "js/chart.min.js" %}"><\/script>');
+ }
+ </script>📝 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.
| <script src="https://cdn.jsdelivr.net/npm/chart.js"></script> | |
| <script src="https://cdn.jsdelivr.net/npm/chart.js"></script> | |
| <script> | |
| // Fallback to local copy if CDN fails | |
| if (typeof Chart === 'undefined') { | |
| document.write('<script src="{% static "js/chart.min.js" %}"><\/script>'); | |
| } | |
| </script> |
🤖 Prompt for AI Agents
In website/templates/dashboard/security_dashboard.html around line 89, the
template currently loads Chart.js directly from a CDN which creates an
availability risk; fix by either (a) adding Chart.js to the project's static
assets and replacing the CDN src with the local static path (e.g., copy the
chart.min.js to your static/js/ folder and reference it via the app's static
URL), or (b) implement a CDN fallback: keep the CDN script tag but add a
fallback loader that detects load failure (onerror or a small inline script that
checks window.Chart) and dynamically injects the local static script if the CDN
fails; also update documentation to note the CDN dependency and the chosen
mitigation.
github-project-automation
bot
moved this from Backlog
to Ready
in 📌 OWASP BLT Project Board
github-actions
bot
added
tests: failed
|
💬 Reminder: Unresolved Conversations This pull request has 4 unresolved conversations that need to be addressed. Please review and resolve the pending discussions so we can move forward with merging this PR. Thank you! 🙏 |
github-actions
bot
added
last-active: 0d
S3DFX-CYBER
left a comment
S3DFX-CYBER
left a comment
There was a problem hiding this comment.
Hey @AyushPatiTripathi, this dashboard looks awesome and is a much-needed feature for BLT! I have a few technical and security-related suggestions to help get this merged:
1.JS Fix: As the Sentry bot noted, you'll need to use document.getElementById for your chart initializations to avoid ReferenceErrors.
2.Security Data Integrity: I'm concerned about the severity mapping in views.py. Mapping null cve_score to 'Low' could be misleading for security teams. We should probably add a When(cve_score__isnull=True, then=Value('Not Scored')) clause to distinguish unrated issues from actual Low-severity ones.
3.Availability: Since we are an OWASP project, relying on an external CDN for chart.js can be a privacy/availability risk. Have you considered hosting the JS file locally in static/js/?
Once those are addressed, I'd be happy to give this an approval!
|
@S3DFX-CYBER Thank you for the review and the helpful suggestions. I agree with all the points you raised — I’ll fix the chart initialization using document.getElementById, update the CVE severity mapping to clearly distinguish “Not Scored” cases, and move Chart.js to a locally hosted static file to avoid CDN dependency. I’ll implement these changes and push an update within the next 24–48 hours. |
github-actions
bot
added
the
changes-requested
| output_field=CharField(), | ||
| ) | ||
| ) | ||
| .values("severity") | ||
| .annotate(count=Count("id")) | ||
| .order_by("-count") | ||
| ) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @dashboard/views.py:
- Around line 60-61: org_qs currently aggregates on domain__name including rows
where domain is null; update the queryset used to build org_qs (the qs variable
pipeline used in org_qs = qs.values("domain__name").annotate(...)) to exclude
null domains (e.g., add .filter(domain__isnull=False) or
.exclude(domain__isnull=True) before calling values()) so that domain__name is
never None in the resulting aggregation.
- Around line 41-55: severity_qs currently classifies a cve_score of 0.0 as
"Low"; update the Case/When mapping in the severity_qs annotate to explicitly
handle score 0 (e.g., add When(cve_score=0, then=Value("None")) as the first
When), change the "Low" branch from When(cve_score__lt=4, then=Value("Low")) to
a bounded range (e.g., When(cve_score__gte=0.1, cve_score__lt=4,
then=Value("Low"))) so 0.0 maps to "None", keep other thresholds the same, and
also update the template/chart legend to include the "None" severity category.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (2)
dashboard/views.pywebsite/templates/dashboard/security_dashboard.html
🚧 Files skipped from review as they are similar to previous changes (1)
- website/templates/dashboard/security_dashboard.html
🧰 Additional context used
🧬 Code graph analysis (1)
dashboard/views.py (1)
website/models.py (1)
Issue(590-739)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run Tests
- GitHub Check: docker-test
🔇 Additional comments (5)
dashboard/views.py (5)
1-9: LGTM!All imports are necessary and correctly used throughout the view function.
12-21: LGTM!The function is properly protected with
@login_required, and the 30-day window calculation is correct. The filter criteria accurately target security issues as specified in the PR objectives.
23-28: LGTM!The KPI calculations correctly compute the total, open, closed, and CVE-related issue counts using appropriate ORM filters.
29-38: LGTM!The daily trend calculation correctly aggregates issues by date and builds a complete 30-day series with zero-filled gaps, ensuring consistent chart rendering.
63-75: LGTM!The context dictionary correctly assembles all computed metrics and the render call properly passes them to the template.
github-actions
bot
added
tests: passed
github-actions
bot
added
files-changed: 8
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @dashboard/views.py:
- Around line 74-75: org_qs currently includes issues with a null domain which
results in entries with domain__name=None; filter the queryset to exclude null
domains before aggregating (e.g., use qs = qs.filter(domain__isnull=False) or qs
= qs.exclude(domain__isnull=True)) so that the
values("domain__name").annotate(...) on org_qs only returns real domain names
and not None.
In @website/static/css/security_dashboard.css:
- Around line 91-93: The global canvas rule is too broad; restrict it to the
dashboard by replacing the bare selector (canvas) with a scoped selector that
targets only dashboard canvases (for example `.security-dashboard canvas` or
`#security-dashboard canvas`), so change the rule in security_dashboard.css from
`canvas { max-height: 240px; }` to a scoped selector like `.security-dashboard
canvas { max-height: 240px; }` (or another specific container class/id used by
the dashboard) to avoid affecting canvases site-wide.
🧹 Nitpick comments (3)
website/static/css/security_dashboard.css (3)
35-53: Consolidate duplicate panel styles.
.paneland.side-panelshare identical properties. Consolidating them reduces duplication and improves maintainability.🔎 Proposed consolidation options
Option 1: Use a single class
-.panel { +.panel, +.side-panel { background: #fff; padding: 14px; border-radius: 12px; box-shadow: 0 2px 10px rgba(0, 0, 0, 0.06); } - -.side-panel { - background: #fff; - padding: 14px; - border-radius: 12px; - box-shadow: 0 2px 10px rgba(0, 0, 0, 0.06); -}Option 2: Use a common base class (if panels may diverge in the future)
+.panel-base { + background: #fff; + padding: 14px; + border-radius: 12px; + box-shadow: 0 2px 10px rgba(0, 0, 0, 0.06); +} + .panel { - background: #fff; - padding: 14px; - border-radius: 12px; - box-shadow: 0 2px 10px rgba(0, 0, 0, 0.06); + /* Panel-specific overrides can go here */ } .side-panel { - background: #fff; - padding: 14px; - border-radius: 12px; - box-shadow: 0 2px 10px rgba(0, 0, 0, 0.06); + /* Side-panel-specific overrides can go here */ }Then update the template to use both classes (e.g.,
class="panel-base panel").
23-33: Consider accessibility for color-only status indicators.KPI card color variants and status dots rely solely on color to convey meaning, which may not be accessible to colorblind users or screen readers (WCAG 2.1 SC 1.4.1).
Consider supplementing color with:
- Icons (e.g., ✓, ⚠, ✕) for KPI cards and dots
- Patterns or shapes for dots (circle, triangle, square)
- ARIA labels or visually-hidden text for screen readers
Example enhancement for dots:
.dot::before { content: ''; /* Add icon font or SVG background */ } .dot[aria-label]::after { content: attr(aria-label); position: absolute; width: 1px; height: 1px; overflow: hidden; clip: rect(0,0,0,0); }Then in the template, add
aria-labelattributes to provide semantic context.Also applies to: 75-89
1-93: Consider using CSS variables or Bootstrap's color system for easier theming.The stylesheet uses hard-coded color values throughout. Leveraging CSS variables or Bootstrap's existing color tokens would improve maintainability and ensure visual consistency with the rest of the application.
Example using CSS variables
Define variables at the top:
:root { --dashboard-bg: #f4f6f9; --panel-bg: #fff; --text-muted: #6c757d; --color-warning: #ffc107; --color-success: #198754; --color-danger: #dc3545; --color-info: #0dcaf0; --shadow-sm: 0 2px 10px rgba(0, 0, 0, 0.06); } .dashboard { background: var(--dashboard-bg); /* ... */ } .kpi-card { background: var(--panel-bg); box-shadow: var(--shadow-sm); /* ... */ }Or reference Bootstrap's CSS variables (e.g.,
var(--bs-warning),var(--bs-success),var(--bs-danger)) if they align with your design intent.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (3)
dashboard/views.pywebsite/static/css/security_dashboard.csswebsite/templates/dashboard/security_dashboard.html
🚧 Files skipped from review as they are similar to previous changes (1)
- website/templates/dashboard/security_dashboard.html
🧰 Additional context used
🧬 Code graph analysis (1)
dashboard/views.py (1)
website/models.py (1)
Issue(590-739)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run Tests
- GitHub Check: Run pre-commit
- GitHub Check: docker-test
🔇 Additional comments (7)
dashboard/views.py (7)
1-9: LGTM!All imports are appropriate and correctly used throughout the view.
12-21: LGTM!The view is properly protected with
@login_required, and the queryset correctly filters security issues from the last 30 days. The date range logic (start_date = today - timedelta(days=29)) produces an inclusive 30-day window.
23-27: LGTM!The KPI calculations are straightforward and correct. The status filters (
"open"and"closed") are hardcoded strings, but this is acceptable assuming these match the Issue model's status field values.
29-39: LGTM!The daily trend logic correctly aggregates issues by day and ensures all 30 days are represented (filling gaps with zeros). The approach is clear and handles missing data appropriately.
41-69: LGTM! Past review concerns have been addressed.The CVSS v3.0 severity mapping is now correct:
- Line 46 explicitly handles
cve_score=0→ "None"- Line 50 uses
cve_score__gt=0to correctly map 0.1–3.9 → "Low"- Lines 56-64 introduce
severity_orderto enforce consistent ordering, addressing the previous color-mismatch concern- Line 68 orders by
severity_orderinstead of countThe logic now properly distinguishes between CVSS 0.0 (None), NULL (Not Scored), and Low severity (0.1–3.9).
71-72: LGTM!The status aggregation is straightforward and correct.
77-89: LGTM!The context dictionary correctly aggregates all computed metrics, and the render call follows Django conventions.
2 participants
his PR introduces a read-only Security Insights Dashboard that provides a consolidated view of security-related issues reported in the last 30 days.
The dashboard leverages existing BLT Issue data and does not introduce any schema or API changes.
Features
KPI overview (total, open, closed, CVE-related issues)
Daily security issue trend (last 30 days)
Issue status distribution
Severity breakdown derived from CVSS score
Top affected organizations/domains
Clean, enterprise-style dashboard layout
Scope & Design Choices
Only issues labeled as Security (label = 4) are included
Data is derived entirely from existing models
Dashboard is read-only
No migrations, no backend breaking changes
UI implemented using existing Bootstrap styles and lightweight Chart.js
Screenshots
Related Issue
Closes #1704
Summary by CodeRabbit
New Features
Security Operations Dashboard added at the app root route: read-only 30-day view with KPIs (total, open, closed, CVE-related), daily trend chart, issue status distribution, severity breakdown, top affected organizations, quick insights panel, and embedded Chart.js visualizations.
Security
Dashboard views require authenticated users (login) to access.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.