fix: don't crash send-report when Slack is configured with a webhook

[breken-ai]

Summary

edr send-report crashes with NotImplementedError when Slack is set up with a webhook (for example slack.notification_webhook in ~/.edr/config.yml, which edr monitor uses) and the report goes to a bucket that returns no website URL (S3/GCS/Azure without --update-bucket-website).

In that setup SlackClient.create_client returns a SlackWebhookClient. send_report uploads the report and posts the summary through the webhook. Because there is no bucket website URL, it then decides to attach the HTML file to Slack and calls SlackWebhookClient.send_report, which is raise NotImplementedError. The command ends with a traceback and a non-zero exit code, even though the upload and the summary both worked.

Fix

DataMonitoringReport.send_report skips the HTML attachment when the Slack client is a webhook client, because a webhook can't upload files, and logs a warning that explains why. Token-based Slack (SlackWebClient) still sends the attachment as before.

Tests

New tests/unit/monitor/data_monitoring/report/test_data_monitoring_report.py, using a DataMonitoringReport subclass in the style of the existing DataMonitoringAlertsMock (the warehouse, the bucket upload and the summary are stubbed; the Slack client is the real one that create_client builds from Config):

• webhook + S3 with no website URL: send_report() returns True. On master (e34d853) it raises NotImplementedError from elementary/clients/slack/client.py:293.
• token + channel + S3: the HTML attachment is still sent to the configured channel.

Full unit suite: 483 passed. black / isort (--profile black) / flake8 / mypy / typos are clean on the changed files.

Summary by CodeRabbit

• Bug Fixes
  • Reports sent through Slack webhooks no longer attempt to attach an HTML file, which webhooks do not support. The report summary is still sent, and the report continues to be uploaded to S3.
  • Slack clients configured with a token continue to receive the HTML report as an attachment in the configured channel.

[github-actions]

👋 @breken-ai
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in this pull request.

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 0b61a37c-afef-461b-bcaf-a566f47ab403

📥 Commits

Reviewing files that changed from the base of the PR and between faa4348 and 20e17e1.

📒 Files selected for processing (1)

• elementary/monitor/data_monitoring/report/data_monitoring_report.py

 ________________________________
< NVIDIA inside, Rabbit outside. >
 --------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 84425ebd-1d8c-4d0a-9ce7-718686d980a0

📥 Commits

Reviewing files that changed from the base of the PR and between a159fed and faa4348.

📒 Files selected for processing (1)

• elementary/monitor/data_monitoring/report/data_monitoring_report.py

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

DataMonitoringReport.send_report skips HTML attachment uploads for Slack webhook clients and logs a warning. Other Slack clients continue to send the attachment. New unit tests cover webhook and token-based Slack configurations.

Changes

Slack report delivery

Layer / File(s)	Summary
Client-specific report delivery elementary/monitor/data_monitoring/report/data_monitoring_report.py, tests/unit/monitor/data_monitoring/report/test_data_monitoring_report.py	Webhook clients skip the HTML attachment and log a warning. Tests check webhook report sending and S3 delivery, plus token-based sending of the local HTML report to the configured channel.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to faa43

Webhook reports avoid the unsupported attachment call while retaining report delivery, and token-based clients still receive attachments. No actionable merge risk is evident.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to a159f

The webhook fix avoids a crash, but a successful Slack summary can make the report appear successful after its bucket upload fails, even though no HTML report was delivered. No new credential or data-access authority was identified.

Retained concerns

• Medium · reliability · inferred: A failed bucket upload followed by a successful webhook summary can return report success without delivering the HTML report. The new skip path allows that result to return normally instead of reaching the previously unsupported attachment call.

Security review details

Security Blast Radius

• inferred — The changed outcome is confined to report sends using an already-configured Slack webhook; the branch does not add a credential source or an external destination. A failed bucket upload can nevertheless be obscured by a successful summary.

Trust Boundaries and Controls

• observed — Configuration still determines the Slack client, bucket clients still perform storage delivery, and the webhook summary still uses the client's message operation. The change removes an unsupported attachment attempt rather than granting webhook file-upload authority.

Resilience and Maintainability Implications

• inferred — Keeping summary-send success separate from storage success matters to failure detection: the current assignment can erase the earlier upload-failure status before the webhook path returns.

Hardening Proposals

• proposed — Keep a bucket-upload failure reflected in the final report result even when the webhook summary succeeds, and exercise that partial-failure path separately from successful delivery.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the fix for crashes when Slack uses a webhook, which is the main change in the pull request.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

_{Comment @coderabbitai help to get the list of available commands.}