AryaKetanShCt opened a new pull request, #42850:
URL: https://github.com/apache/superset/pull/42850
### SUMMARY
A report or alert saved with a Slack **channel name** rather than a channel
**id** validates cleanly, runs on schedule, and then never delivers.
`ReportRecipientConfigJSONSchema.target` is a bare `fields.String()`, and
the only validator on `ReportRecipientSchema` begins:
```python
if data.get("type") != ReportRecipientType.EMAIL.value:
return
```
So email addresses are regex-checked and Slack targets are not checked at
all. `POST /api/v1/report/` accepts `{"type": "SlackV2",
"recipient_config_json": {"target": "data-alerts"}}` and returns 201.
But `SlackV2Notification.send()` passes the target straight to
`files_upload_v2(channel=...)`, which requires a channel id and answers
`invalid_arguments` for a name. The only signal is an error notification to the
report's owner, long after the alert was created, and only if someone reads it.
The UI is unaffected because its picker submits ids from
`/api/v1/report/slack_channels/`. The gap is the API.
This is not theoretical. On one instance 46 recipients had accumulated a
channel name, 21 of them on active alerts, the oldest from 2023. Of the 19
distinct names, 5 still matched a live channel, 2 matched archived ones and the
rest matched nothing at all, so most could not be repaired even by hand and 15
alerts had to be switched off.
### FIX
Validate Slack and SlackV2 targets the way Email targets are already
validated: every comma, semicolon or whitespace separated part must look like a
channel id (`^[CGD][A-Z0-9]{6,}$`). The error names the offending part and says
where to find the id, rather than only saying the value is invalid.
The message is deliberately explicit about *why*, because "invalid channel"
reads like a typo when the real cause is that names are accepted by Slack's
older `chat.postMessage` and not by the upload API used for attachments.
### TESTING INSTRUCTIONS
Six unit tests in `tests/unit_tests/reports/schemas_test.py`, following the
existing email cases: a single id and multiple ids are accepted; a name, a
mixed id-and-name target, and an empty target are rejected; the legacy `Slack`
type is validated the same way. The mixed case asserts the message names the
bad part and not the good one.
Manually: `POST /api/v1/report/` with a Slack recipient whose target is a
channel name now returns 400 instead of 201.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
