feat(seer-infra-telemetry): PAT modal builder + submission handler for monitoring providers#118607
Conversation
github-actions
Bot
added
the
Scope: Backend
|
bugbot run |
| MONITORING_PROVIDERS: dict[str, dict[str, Any]] = { | ||
| "datadog": {"name": "Datadog"}, | ||
| "datadog_pat": {"name": "Datadog (Personal Access Token)"}, | ||
| "datadog_pat": { |
There was a problem hiding this comment.
Lmk what y'all think about this structure for storing provider-specific configuration for Slack flows. Alternatively we could just add some branching in the Slack message builder to handle providers separately (for the blocks that are different)
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| "datadog_pat": {"name": "Datadog (Personal Access Token)"}, | ||
| "datadog_pat": { | ||
| "name": "Datadog (Personal Access Token)", | ||
| "pat_hint": "Create one at Organization Settings → Access → API Keys in Datadog.", |
There was a problem hiding this comment.
We'll eventually want more instruction/detail than this (and probably displayed in a more readable/understandable way). e.g. we'll want to indicate which scopes/expiration to set for the token (all read scopes, max of 1 year). I'm thinking this is okay for now for testing.
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d46e6f8. Configure here.
| build_data["site"] = site | ||
|
|
||
| try: | ||
| identity_data = provider_type.build_identity(build_data) |
There was a problem hiding this comment.
OAuth provider submission uncaught error
High Severity
handle_monitoring_provider_submission calls build_identity for any key in MONITORING_PROVIDERS without rejecting OAuth providers, unlike open_monitoring_provider_modal. A crafted modal submission with provider_key datadog passes validation but triggers an uncaught KeyError inside OAuth build_identity, returning 500 instead of a modal error.
Reviewed by Cursor Bugbot for commit d46e6f8. Configure here.
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
| @@ -756,6 +762,24 @@ def post(self, request: Request) -> Response: | |||
| if action_option in NOTIFICATION_SETTINGS_ACTION_OPTIONS: | |||
There was a problem hiding this comment.
User-submitted PAT logged to application logs via slack.action.request
The monitoring-provider modal submits a third-party personal access token in view.state.values.token_block.token_input.value, which is part of slack_request.data. The post() handler logs the entire slack_request.data at INFO level (slack.action.request) before dispatching the new view_submission handler, so every PAT submission is written to application logs in cleartext. Redact the token field before logging.
Evidence
post()inaction.pylogsextra={... 'request_data': slack_request.data}at INFO unconditionally (line ~719) before any action dispatch.- The new
view_submissionintercept forMONITORING_PROVIDER_CALLBACK_IDruns after this log, so the payload is already logged. monitoring_provider.py:246reads the PAT fromstate_values.token_block.token_input.value, the same value present in the loggedslack_request.data.- No redaction or field exclusion is applied to
request_data, exposing users' third-party tokens to anyone with log access.
Identified by Warden security-review · L2P-KS2
| slack_client.chat_postEphemeral( | ||
| channel=channel_id, | ||
| user=user_id, | ||
| text=f"Connected {provider_name}. Seer can now query {provider_name} data in this org.", |
There was a problem hiding this comment.
SlackApiError from chat_postEphemeral propagates after successful identity link
If chat_postEphemeral raises SlackApiError (e.g. channel_not_found, user_not_in_channel), the exception propagates out of handle_monitoring_provider_submission after link_provider_identity has already committed — the user sees a modal error but is permanently linked, so any retry hits "This account is already connected". Wrap the call in a try/except SlackApiError and log the failure instead.
Evidence
SlackSdkClient.wrapperinsdk_client.pyre-raisesSlackApiErrorafter logging, so all SDK calls includingchat_postEphemeralcan raise.workspace.pyline 158 wraps the identicalchat_postEphemeralcall intry/except SlackApiError, confirming it is a known failure mode.link_provider_identityat line 271 succeeds and commits the DB change before_send_success_ephemeralis called at line 286._send_success_ephemeralhas no exception handling;SlackApiErrorpropagates all the way out ofhandle_monitoring_provider_submission.- A subsequent retry by the user then hits the
IntegrityErrorguard at line 280 and returns "This account is already connected".
Identified by Warden sentry-backend-bugs · ZBR-37R
1 participant
Resolves CW-1549.
Adds a Slack modal for PAT-based monitoring provider connection (org selector, site dropdown, token input) opened from button click. The
view_submissionhandler validates the token via the provider's API, creates the necessary identity-linked objects via thelink_provider_identity()helper, clears any prior decline, and sends an ephemeral success message.The new Slack actions are:
CONNECT_MONITORING_PROVIDER,SKIP_MONITORING_PROVIDER, andDECLINE_MONITORING_PROVIDER.SKIP_MONITORING_PROVIDERskips the provider connection prompt for the rest of this run/thread, whileDECLINE_MONITORING_PROVIDERprevents showing the provider connection prompt to the same user (based on Slack identity) permanently.All monitoring provider functionality is gated behind the
organizations:seer-infra-telemetryfeature flag.https://claude.ai/code/artifact/f54a82fa-73ca-49ef-a331-6f57cf26508d