feat: add webhook notifications on scan completion#326
feat: add webhook notifications on scan completion#3260xhis wants to merge 4 commits intousestrix:mainfrom
Conversation
Add support for sending scan results to external services when a penetration test completes. Supports three payload formats: - Generic JSON (raw vulnerability data + stats) - Slack (Block Kit message with severity breakdown) - Discord (Rich Embed with color-coded severity) Configuration via CLI args (--webhook-url, --webhook-format) or environment variables (STRIX_WEBHOOK_URL, STRIX_WEBHOOK_FORMAT). Auto-detects Slack/Discord from the webhook URL hostname when format is set to "generic". Includes 22 unit tests covering format resolution, all three formatters, helper functions, and the send function with mocked HTTP.
Greptile SummaryAdded webhook notifications on scan completion with support for generic JSON, Slack, and Discord formats. The implementation includes automatic format detection from URL patterns, CLI arguments and environment variables for configuration, and comprehensive error handling with non-blocking failures. Key changes:
The feature integrates cleanly with existing code patterns and follows the project's style guidelines. All tests pass and the implementation handles edge cases properly (empty vulnerability lists, scan failures, network errors). Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 8fdfb9c |
There was a problem hiding this comment.
Pull request overview
This PR adds webhook notification functionality to Strix, allowing scan results to be automatically pushed to external services upon completion. The implementation supports three payload formats: generic JSON, Slack Block Kit, and Discord embeds, with auto-detection based on webhook URLs.
Changes:
- Added comprehensive webhook dispatcher module with three formatters and URL-based format detection
- Integrated webhook configuration into Config class and CLI arguments
- Added webhook dispatch call in main() after scan completion
- Provided extensive unit test coverage with 22 tests
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| strix/interface/webhooks.py | New module implementing webhook dispatcher with formatters for generic, Slack, and Discord formats |
| strix/config/config.py | Added strix_webhook_url and strix_webhook_format configuration fields |
| strix/interface/main.py | Integrated webhook CLI arguments and dispatch logic after scan completion |
| tests/interface/test_webhooks.py | Comprehensive test suite with 22 unit tests covering all formatters and edge cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| webhook_url = args.webhook_url or Config.get("strix_webhook_url") | ||
| if webhook_url: | ||
| webhook_format = args.webhook_format or Config.get("strix_webhook_format") or "generic" | ||
| send_completion_webhook( | ||
| webhook_url=webhook_url, | ||
| webhook_format=webhook_format, | ||
| tracer=tracer, | ||
| args=args, | ||
| ) |
There was a problem hiding this comment.
The tracer variable is obtained from get_global_tracer() in the finally block at line 592, which can return None. However, it's passed to send_completion_webhook without checking if it's None first. This could lead to AttributeError when the webhook formatters try to access tracer attributes like tracer.vulnerability_reports or tracer.agents. Add a check to ensure tracer is not None before calling send_completion_webhook.
| def _format_generic(tracer: Any, args: argparse.Namespace) -> dict[str, Any]: | ||
| """Plain JSON payload with full scan data.""" | ||
| completed = _scan_completed(tracer) | ||
| llm_stats = tracer.get_total_llm_stats()["total"] if tracer else {} | ||
| return { | ||
| "event": "scan_completed" if completed else "scan_ended", | ||
| "run_name": getattr(args, "run_name", ""), | ||
| "targets": _targets_summary(args), | ||
| "scan_mode": getattr(args, "scan_mode", ""), | ||
| "completed": completed, | ||
| "vulnerability_count": len(tracer.vulnerability_reports), | ||
| "severity_counts": _severity_counts(tracer), | ||
| "vulnerabilities": _vulnerability_summary(tracer), | ||
| "stats": { | ||
| "agents": len(tracer.agents), | ||
| "tools": tracer.get_real_tool_count(), | ||
| "input_tokens": llm_stats.get("input_tokens", 0), | ||
| "output_tokens": llm_stats.get("output_tokens", 0), | ||
| "cost": llm_stats.get("cost", 0), | ||
| }, | ||
| } |
There was a problem hiding this comment.
The formatters need to handle the case where tracer is None more consistently. Currently, _format_generic checks if tracer is truthy for llm_stats (line 132) but then accesses tracer.vulnerability_reports (line 139), tracer.agents (line 143), and tracer.get_real_tool_count() (line 144) without checking. If tracer is None, these will raise AttributeError. Either add None checks for all tracer accesses, or ensure the function is never called with None.
| def _format_slack(tracer: Any, args: argparse.Namespace) -> dict[str, Any]: | ||
| """Slack Block Kit payload.""" | ||
| completed = _scan_completed(tracer) | ||
| vuln_count = len(tracer.vulnerability_reports) | ||
| counts = _severity_counts(tracer) | ||
|
|
||
| status_emoji = ":white_check_mark:" if completed else ":warning:" | ||
| status_text = "completed" if completed else "ended" | ||
|
|
||
| severity_line = ( | ||
| " | ".join(f"*{sev.upper()}*: {cnt}" for sev, cnt in counts.items() if cnt > 0) | ||
| or "None found" | ||
| ) | ||
|
|
||
| blocks: list[dict[str, Any]] = [ | ||
| { | ||
| "type": "header", | ||
| "text": { | ||
| "type": "plain_text", | ||
| "text": f"{status_emoji} Strix Scan {status_text.title()}", | ||
| "emoji": True, | ||
| }, | ||
| }, | ||
| { | ||
| "type": "section", | ||
| "fields": [ | ||
| {"type": "mrkdwn", "text": f"*Target:*\n{_targets_summary(args)}"}, | ||
| {"type": "mrkdwn", "text": f"*Run:*\n{getattr(args, 'run_name', 'N/A')}"}, | ||
| {"type": "mrkdwn", "text": f"*Scan Mode:*\n{getattr(args, 'scan_mode', 'N/A')}"}, | ||
| {"type": "mrkdwn", "text": f"*Vulnerabilities:*\n{vuln_count}"}, | ||
| ], | ||
| }, | ||
| { | ||
| "type": "section", | ||
| "text": { | ||
| "type": "mrkdwn", | ||
| "text": f"*Severity Breakdown:* {severity_line}", | ||
| }, | ||
| }, | ||
| ] | ||
|
|
||
| # Add top vulnerabilities (max 5) | ||
| for report in tracer.vulnerability_reports[:5]: | ||
| title = report.get("title", "Untitled") | ||
| severity = report.get("severity", "unknown").upper() | ||
| endpoint = report.get("endpoint", "") | ||
| text = f":rotating_light: *[{severity}]* {title}" | ||
| if endpoint: | ||
| text += f"\n`{endpoint}`" | ||
| blocks.append( | ||
| { | ||
| "type": "section", | ||
| "text": {"type": "mrkdwn", "text": text}, | ||
| } | ||
| ) | ||
|
|
||
| return {"blocks": blocks} |
There was a problem hiding this comment.
Similar to _format_generic, this function accesses tracer.vulnerability_reports and tracer.agents without checking if tracer is None. Add None checks to prevent AttributeError when tracer is None.
| class TestSendCompletionWebhook: | ||
| """Tests for the top-level send function.""" | ||
|
|
||
| @patch("strix.interface.webhooks.requests.post") | ||
| def test_posts_to_url(self, mock_post: MagicMock) -> None: | ||
| """Verify that the function POSTs to the provided URL.""" | ||
| mock_post.return_value = MagicMock(status_code=200) | ||
| tracer = _make_tracer() | ||
| args = _make_args() | ||
|
|
||
| send_completion_webhook("https://example.com/hook", "generic", tracer, args) | ||
|
|
||
| mock_post.assert_called_once() | ||
| call_kwargs = mock_post.call_args | ||
| assert call_kwargs[1]["json"]["event"] == "scan_completed" | ||
|
|
||
| @patch("strix.interface.webhooks.requests.post") | ||
| def test_auto_detects_slack(self, mock_post: MagicMock) -> None: | ||
| """Verify Slack auto-detection produces Block Kit payload.""" | ||
| mock_post.return_value = MagicMock(status_code=200) | ||
| tracer = _make_tracer() | ||
| args = _make_args() | ||
|
|
||
| send_completion_webhook( | ||
| "https://hooks.slack.com/services/T00/B00/xxx", "generic", tracer, args | ||
| ) | ||
|
|
||
| payload = mock_post.call_args[1]["json"] | ||
| assert "blocks" in payload | ||
|
|
||
| @patch("strix.interface.webhooks.requests.post") | ||
| def test_failure_does_not_raise(self, mock_post: MagicMock) -> None: | ||
| """Webhook delivery failures should be logged, not raised.""" | ||
| mock_post.side_effect = requests.ConnectionError("refused") | ||
| tracer = _make_tracer() | ||
| args = _make_args() | ||
|
|
||
| # Should not raise | ||
| send_completion_webhook("https://example.com/hook", "generic", tracer, args) |
There was a problem hiding this comment.
The tests don't cover the edge case where tracer is None, which is possible in the actual usage (see main.py line 605). Add test cases that verify the behavior when tracer is None to ensure the formatters handle this case gracefully without raising AttributeError.
strix/interface/webhooks.py
Outdated
| for report in tracer.vulnerability_reports[:5]: | ||
| title = report.get("title", "Untitled") | ||
| severity = report.get("severity", "unknown").upper() | ||
| endpoint = report.get("endpoint", "") | ||
| value = f"**[{severity}]** {title}" | ||
| if endpoint: | ||
| value += f"\n`{endpoint}`" | ||
| fields.append({"name": "\u200b", "value": value, "inline": False}) |
There was a problem hiding this comment.
Discord has limits on embed field values (1024 characters) and titles (256 characters). The vulnerability title and endpoint values are directly inserted into fields without truncation. If a vulnerability has a very long title or endpoint, the Discord webhook may fail. Consider truncating these values to stay within Discord's limits.
| def _format_discord(tracer: Any, args: argparse.Namespace) -> dict[str, Any]: | ||
| """Discord webhook payload with an embed.""" | ||
| completed = _scan_completed(tracer) | ||
| vuln_count = len(tracer.vulnerability_reports) | ||
| counts = _severity_counts(tracer) | ||
|
|
||
| color = 0x22C55E if completed else 0xEAB308 # green / yellow | ||
| if counts["critical"] > 0: | ||
| color = 0xDC2626 | ||
| elif counts["high"] > 0: | ||
| color = 0xEA580C | ||
|
|
||
| severity_line = ( | ||
| " | ".join(f"**{sev.upper()}**: {cnt}" for sev, cnt in counts.items() if cnt > 0) | ||
| or "None found" | ||
| ) | ||
|
|
||
| fields: list[dict[str, Any]] = [ | ||
| {"name": "Target", "value": _targets_summary(args), "inline": True}, | ||
| {"name": "Scan Mode", "value": getattr(args, "scan_mode", "N/A"), "inline": True}, | ||
| {"name": "Vulnerabilities", "value": str(vuln_count), "inline": True}, | ||
| {"name": "Severity Breakdown", "value": severity_line, "inline": False}, | ||
| ] | ||
|
|
||
| # Top vulnerabilities (max 5) | ||
| for report in tracer.vulnerability_reports[:5]: | ||
| title = report.get("title", "Untitled") | ||
| severity = report.get("severity", "unknown").upper() | ||
| endpoint = report.get("endpoint", "") | ||
| value = f"**[{severity}]** {title}" | ||
| if endpoint: | ||
| value += f"\n`{endpoint}`" | ||
| fields.append({"name": "\u200b", "value": value, "inline": False}) | ||
|
|
||
| status_text = "Scan Completed" if completed else "Scan Ended" | ||
|
|
||
| embed: dict[str, Any] = { | ||
| "title": f"\ud83d\udd12 Strix \u2014 {status_text}", | ||
| "description": f"Run: **{getattr(args, 'run_name', 'N/A')}**", | ||
| "color": color, | ||
| "fields": fields, | ||
| "footer": {"text": "Strix Security Scanner"}, | ||
| } | ||
|
|
||
| return {"embeds": [embed]} |
There was a problem hiding this comment.
Similar to other formatters, this function accesses tracer.vulnerability_reports without checking if tracer is None. Add None checks to prevent AttributeError when tracer is None.
| try: | ||
| response = requests.post(webhook_url, json=payload, timeout=WEBHOOK_TIMEOUT) |
There was a problem hiding this comment.
The webhook URL is not validated before being used with requests.post. While webhook URLs are configured by the user (not untrusted input), it would be more defensive to validate that the URL has a proper scheme (http/https) to prevent unexpected behavior. Consider adding URL validation to check the scheme before making the request.
strix/interface/webhooks.py
Outdated
| for report in tracer.vulnerability_reports[:5]: | ||
| title = report.get("title", "Untitled") | ||
| severity = report.get("severity", "unknown").upper() | ||
| endpoint = report.get("endpoint", "") | ||
| text = f":rotating_light: *[{severity}]* {title}" | ||
| if endpoint: | ||
| text += f"\n`{endpoint}`" | ||
| blocks.append( | ||
| { | ||
| "type": "section", | ||
| "text": {"type": "mrkdwn", "text": text}, | ||
| } | ||
| ) |
There was a problem hiding this comment.
Slack Block Kit has limits on text content in blocks. Section blocks can have a maximum of 3000 characters in the text field. The vulnerability title and endpoint values are directly inserted without truncation. If a vulnerability has very long values, the Slack webhook may fail. Consider truncating these values to stay within Slack's limits.
strix/interface/webhooks.py
Outdated
| "title": f"\ud83d\udd12 Strix \u2014 {status_text}", | ||
| "description": f"Run: **{getattr(args, 'run_name', 'N/A')}**", |
There was a problem hiding this comment.
Discord embed descriptions have a limit of 4096 characters. The run_name is inserted directly into the description without truncation. If the run_name is extremely long, this could potentially cause issues, though run_name is typically short. Consider adding a reasonable length check or truncation for defensive programming.
- Validate webhook URL scheme (http/https only) - Guard against tracer=None in all formatters and helpers - Truncate long strings for Slack (3000 char) and Discord (1024 char) limits - Add _truncate helper with ellipsis - Add 11 new tests for None tracer, URL validation, and truncation
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Add webhook notifications on scan completion, allowing Strix to push results to external services automatically. This is useful for CI/CD pipelines and team alerting workflows.
Supported Formats
Configuration
CLI arguments (highest priority):
Environment variables:
Auto-detection
When
--webhook-formatisgeneric(default), the format is auto-detected from the URL:hooks.slack.com→ Slack Block Kitdiscord.com/discordapp.com→ Discord EmbedChanges
strix/interface/webhooks.py[NEW] — Dispatcher with 3 formatters, URL-based format detectionstrix/config/config.py— Addedstrix_webhook_urlandstrix_webhook_formatconfig fieldsstrix/interface/main.py— Added--webhook-url/--webhook-formatCLI args, dispatch after scantests/interface/test_webhooks.py[NEW] — 22 unit tests covering all formatters, helpers, and edge casesDesign Decisions
requests(already a dependency), no new packages neededConfig.get(), CLI viaargparse, same code styleTesting
All 22 unit tests pass:
ruff checkandruff formatpass on all changed files.