feat: log unsafe queries to Slack for better monitoring#1651
Conversation
There was a problem hiding this comment.
Pull request overview
Adds Slack notifications when validateQuerySafety rejects a query, to improve monitoring of unsafe/read-write query attempts across panel creation/execution and AI-generated panels.
Changes:
- Import and invoke
slackPostMessagewhen a query is deemed unsafe. - Include query text, rejection reason, and connection type in the Slack message.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,5 +1,6 @@ | |||
| import { BadRequestException } from '@nestjs/common'; | |||
| import { ConnectionTypesEnum } from '@rocketadmin/shared-code/dist/src/shared/enums/connection-types-enum.js'; | |||
| import { slackPostMessage } from '../../../../helpers/index.js'; | |||
There was a problem hiding this comment.
Importing slackPostMessage into this low-level query-safety utility introduces a network/IO side effect and a dependency on the Slack helper into code that otherwise acts like a pure validator. This makes the function harder to reuse/test and also prevents callers from attaching context (e.g., userId/connectionId) or choosing different reporting behavior. Consider keeping validateQuerySafety side-effect free and moving the Slack notification to the use-cases/controllers (or injecting an optional reporter callback).
| import { slackPostMessage } from '../../../../helpers/index.js'; |
| slackPostMessage(`Unsafe query: ${query}\nReason: ${result.reason}\nConnection Type: ${connectionType}`); | ||
| throw new BadRequestException(`Unsafe query: ${result.reason}. Only read-only queries are allowed.`); |
There was a problem hiding this comment.
This posts the full raw query text to Slack. Since queries can contain sensitive information (table/column names, tenant identifiers, literal values, etc.) and are user-controlled, this can cause unintended data exposure and may violate privacy/compliance expectations. Prefer logging a redacted/truncated form (e.g., remove string/numeric literals, cap length) and/or a hash + metadata, and consider a dedicated Slack channel or feature-flagging this behavior.
| slackPostMessage(`Unsafe query: ${query}\nReason: ${result.reason}\nConnection Type: ${connectionType}`); | ||
| throw new BadRequestException(`Unsafe query: ${result.reason}. Only read-only queries are allowed.`); |
There was a problem hiding this comment.
Posting a Slack message for every rejected query can be very noisy and can be abused to generate high Slack volume (and outbound HTTP traffic) by repeatedly submitting unsafe queries. Consider adding throttling/sampling, limiting to certain request sources (e.g., AI-generated queries only), or guarding with an environment flag so it can be enabled for monitoring without impacting normal operation.
No description provided.