mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-12 16:02:56 +00:00
fix: dispatch Slack agents only on a verified request, warn when the connection lacks the agent scopes, register the callback redirect URLs
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
d0ef9dee91
commit
7400577481
@@ -1 +1 @@
|
||||
e6c86a632b05cc0561392cb8cdbe9e63f2ae4f28
|
||||
6f1002bb0a8d36f674b6f1d0865daf985e83ede9
|
||||
|
||||
@@ -13474,6 +13474,11 @@ paths:
|
||||
bot_name:
|
||||
type: string
|
||||
description: the bot's Slack handle; absent when Slack can't be reached
|
||||
missing_scopes:
|
||||
type: array
|
||||
items:
|
||||
type: string
|
||||
description: agent scopes the workspace's Slack connection lacks, so it must be reconnected with AI agents enabled
|
||||
channels:
|
||||
type: array
|
||||
items:
|
||||
@@ -13481,6 +13486,7 @@ paths:
|
||||
required:
|
||||
- channels
|
||||
- signing_secret_set
|
||||
- missing_scopes
|
||||
|
||||
/w/{workspace}/flow_conversations/update/{conversation_id}:
|
||||
post:
|
||||
|
||||
@@ -264,7 +264,6 @@ lazy_static::lazy_static! {
|
||||
/// Added to `SLACK_OAUTH_SCOPES` when a workspace connects with AI agents enabled: who sent a
|
||||
/// message (by email) and the channel picker. Opt-in, as a Slack app must declare every scope
|
||||
/// it is asked for.
|
||||
#[cfg(feature = "oauth2")]
|
||||
pub const SLACK_AGENT_SCOPES: &[&str] = &["users:read", "users:read.email", "channels:read"];
|
||||
|
||||
/// The env var wins over the instance setting.
|
||||
|
||||
@@ -86,6 +86,8 @@ struct AgentSlack {
|
||||
signing_secret_set: bool,
|
||||
/// The bot's Slack handle, what people mention; `None` when Slack can't be reached.
|
||||
bot_name: Option<String>,
|
||||
/// Agent scopes the workspace's connection lacks: it was made without AI agents enabled.
|
||||
missing_scopes: Vec<String>,
|
||||
channels: Vec<ChannelAgent>,
|
||||
}
|
||||
|
||||
@@ -119,29 +121,51 @@ async fn list_agent_channels(
|
||||
let signing_secret_set = crate::slack_signing_secret().is_some();
|
||||
#[cfg(not(feature = "oauth2"))]
|
||||
let signing_secret_set = false;
|
||||
let bot_name = match slack_team_name {
|
||||
Some(_) => bot_name(&db, &w_id).await,
|
||||
None => None,
|
||||
let (bot_name, missing_scopes) = match slack_team_name {
|
||||
Some(_) => bot_identity(&db, &w_id).await.unzip(),
|
||||
None => (None, None),
|
||||
};
|
||||
Ok(Json(AgentSlack {
|
||||
slack_team_name,
|
||||
signing_secret_set,
|
||||
bot_name,
|
||||
missing_scopes: missing_scopes.unwrap_or_default(),
|
||||
channels,
|
||||
}))
|
||||
}
|
||||
|
||||
async fn bot_name(db: &DB, w_id: &str) -> Option<String> {
|
||||
/// The bot's handle and the agent scopes its token lacks; `None` when Slack can't be reached.
|
||||
async fn bot_identity(db: &DB, w_id: &str) -> Option<(String, Vec<String>)> {
|
||||
let token = bot_token(db, w_id).await.ok()?;
|
||||
let call = async {
|
||||
let res = HTTP_CLIENT
|
||||
.post("https://slack.com/api/auth.test")
|
||||
.bearer_auth(&token)
|
||||
.send()
|
||||
.await
|
||||
.ok()?;
|
||||
// Slack lists the token's scopes in this header on every Web API response.
|
||||
let granted = res
|
||||
.headers()
|
||||
.get("x-oauth-scopes")
|
||||
.and_then(|v| v.to_str().ok())
|
||||
.unwrap_or_default()
|
||||
.split(',')
|
||||
.map(|s| s.trim().to_string())
|
||||
.collect::<Vec<_>>();
|
||||
let body: serde_json::Value = res.json().await.ok()?;
|
||||
let name = body["user"].as_str()?.to_string();
|
||||
let missing = crate::SLACK_AGENT_SCOPES
|
||||
.iter()
|
||||
.filter(|s| !granted.iter().any(|g| g == *s))
|
||||
.map(|s| s.to_string())
|
||||
.collect();
|
||||
Some((name, missing))
|
||||
};
|
||||
// On every agent page load: a slow Slack must not hold the page for the client's 20s timeout.
|
||||
let res = tokio::time::timeout(
|
||||
std::time::Duration::from_secs(3),
|
||||
slack_call(&token, "auth.test", &[]),
|
||||
)
|
||||
.await
|
||||
.ok()?
|
||||
.ok()?;
|
||||
res["user"].as_str().map(str::to_string)
|
||||
tokio::time::timeout(std::time::Duration::from_secs(3), call)
|
||||
.await
|
||||
.ok()?
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
@@ -330,6 +354,9 @@ pub struct SlackMessage {
|
||||
pub thread_ts: Option<String>,
|
||||
/// A slash command's, where it can be answered when the bot cannot post in the channel.
|
||||
pub response_url: Option<String>,
|
||||
/// Whether this request's Slack signature was verified. Carried from the check rather than
|
||||
/// re-read from the setting, which can be enabled between the two.
|
||||
pub signed: bool,
|
||||
}
|
||||
|
||||
enum Route {
|
||||
@@ -344,8 +371,7 @@ enum Route {
|
||||
pub async fn try_answer(db: &DB, msg: SlackMessage) -> Result<bool> {
|
||||
// Without it the Slack endpoints take unsigned requests, and the sender's `user_id` would
|
||||
// pick whose permissions the agent runs with.
|
||||
#[cfg(feature = "oauth2")]
|
||||
if crate::slack_signing_secret().is_none() {
|
||||
if !msg.signed {
|
||||
return Ok(false);
|
||||
}
|
||||
let text = decode_entities(&msg.text);
|
||||
|
||||
@@ -140,7 +140,8 @@
|
||||
<span class="text-2xs text-secondary">
|
||||
Under Basic Information in the Slack app. Windmill checks with it that requests come
|
||||
from Slack, and AI agents answer in Slack only once it is set. The SLACK_SIGNING_SECRET
|
||||
environment variable takes precedence.
|
||||
environment variable takes precedence. A workspace that uses its own Slack app is
|
||||
checked against this secret too, so its Slack requests fail unless it is the same app.
|
||||
</span>
|
||||
<Password
|
||||
id="slack_signing_secret"
|
||||
|
||||
@@ -26,7 +26,8 @@ features:
|
||||
should_escape: false
|
||||
oauth_config:
|
||||
redirect_urls:
|
||||
- ${baseUrl}
|
||||
- ${baseUrl}/oauth/callback_slack
|
||||
- ${baseUrl}/oauth/callback_slack/instance
|
||||
scopes:
|
||||
bot:
|
||||
- chat:write
|
||||
|
||||
@@ -37,6 +37,7 @@
|
||||
slackLoading,
|
||||
slackTeamName,
|
||||
botName,
|
||||
missingScopes,
|
||||
signingSecretSet,
|
||||
channels,
|
||||
onChanged
|
||||
@@ -52,6 +53,8 @@
|
||||
slackTeamName: string | undefined
|
||||
/** Unset when Slack can't be reached with the workspace's bot token. */
|
||||
botName: string | undefined
|
||||
/** Agent scopes the connection lacks: Slack was connected without AI agents enabled. */
|
||||
missingScopes: string[]
|
||||
/** Agents answer in Slack only when the instance verifies Slack's request signatures. */
|
||||
signingSecretSet: boolean
|
||||
channels: SlackChannelAgent[]
|
||||
@@ -69,7 +72,13 @@
|
||||
// Read again whenever the agent's channels change, so a channel just added or removed moves
|
||||
// between the list and the picker.
|
||||
const available = resource(
|
||||
() => ({ ws: isAdmin && chat && slackTeamName && botName ? workspace : undefined, channels }),
|
||||
() => ({
|
||||
ws:
|
||||
isAdmin && chat && slackTeamName && botName && missingScopes.length === 0
|
||||
? workspace
|
||||
: undefined,
|
||||
channels
|
||||
}),
|
||||
async ({ ws }) => (ws ? await listChoices(ws) : [])
|
||||
)
|
||||
|
||||
@@ -215,6 +224,18 @@
|
||||
{/if}
|
||||
</Alert>
|
||||
{/if}
|
||||
{#if botName && missingScopes.length > 0}
|
||||
<Alert type="warning" title="Slack was connected without AI agents" size="xs">
|
||||
The connection lacks {missingScopes.join(', ')}, so agents can't tell who is asking.
|
||||
{#if isAdmin}
|
||||
<a href="{base}/workspace_settings?tab=slack&agents=true">Disconnect Slack</a>,
|
||||
then connect it again with AI agents enabled. Disconnecting clears the channel
|
||||
defaults.
|
||||
{:else}
|
||||
A workspace admin can reconnect Slack with AI agents enabled.
|
||||
{/if}
|
||||
</Alert>
|
||||
{/if}
|
||||
<div class="flex flex-col gap-1">
|
||||
<span class="text-xs font-semibold text-emphasis">Ask it anywhere</span>
|
||||
<span class="text-xs text-secondary">
|
||||
@@ -260,7 +281,7 @@
|
||||
{/each}
|
||||
</div>
|
||||
{/if}
|
||||
{#if isAdmin}
|
||||
{#if isAdmin && missingScopes.length === 0}
|
||||
<div class="flex items-center gap-2">
|
||||
<Select
|
||||
class="grow"
|
||||
@@ -280,7 +301,7 @@
|
||||
Add
|
||||
</Button>
|
||||
</div>
|
||||
{:else}
|
||||
{:else if !isAdmin}
|
||||
<span class="text-2xs text-hint">Only workspace admins can change these.</span>
|
||||
{/if}
|
||||
</div>
|
||||
|
||||
@@ -217,6 +217,7 @@
|
||||
slackLoading={slack.loading && !slack.current}
|
||||
slackTeamName={slack.current?.slack_team_name}
|
||||
botName={slack.current?.bot_name}
|
||||
missingScopes={slack.current?.missing_scopes ?? []}
|
||||
signingSecretSet={slack.current?.signing_secret_set ?? true}
|
||||
channels={slack.current?.channels ?? []}
|
||||
onChanged={() => slack.refetch()}
|
||||
|
||||
Reference in New Issue
Block a user