From a1b9f900c7e1a9f9e474964082d3f743b05fcbc1 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sat, 23 May 2026 14:15:22 -0400 Subject: [PATCH] perf: Avoid correlated subquery in Slack hooks user lookup (#12432) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * perf: Avoid correlated subquery in Slack hooks user lookup Query UserAuthentication directly by indexed providerId and load the associated User and Team, instead of driving from User.findOne with a required hasMany include — which Sequelize translates into a correlated subquery that scans the users table. Co-Authored-By: Claude Opus 4.7 * fix: Scope Slack fallback user lookup to matching AuthenticationProvider The fallback in findUserForRequest matched any UserAuthentication with the same providerId, which is only unique per (providerId, userId). A colliding external user id from another workspace or provider could resolve a user from the wrong team. Constrain via the AuthenticationProvider join (name = "slack", providerId = serviceTeamId). Co-Authored-By: Claude Opus 4.7 * test --------- Co-authored-by: Claude Opus 4.7 --- plugins/slack/server/api/hooks.test.ts | 2 +- plugins/slack/server/api/hooks.ts | 37 +++++++++++++++++--------- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/plugins/slack/server/api/hooks.test.ts b/plugins/slack/server/api/hooks.test.ts index 35769ce1ab..335bd51726 100644 --- a/plugins/slack/server/api/hooks.test.ts +++ b/plugins/slack/server/api/hooks.test.ts @@ -34,7 +34,7 @@ describe("#hooks.unfurl", () => { const res = await server.post("/api/hooks.unfurl", { body: { token: env.SLACK_VERIFICATION_TOKEN, - team_id: `T${randomString(8)}`, + team_id: user.team.authenticationProviders[0].providerId, api_app_id: `A${randomString(8)}`, event: { type: "link_shared", diff --git a/plugins/slack/server/api/hooks.ts b/plugins/slack/server/api/hooks.ts index ba656bffb8..95a86873cd 100644 --- a/plugins/slack/server/api/hooks.ts +++ b/plugins/slack/server/api/hooks.ts @@ -403,28 +403,41 @@ async function findUserForRequest( return integration.user; } - // Fallback to authentication provider if the user has Slack sign-in - const user = await User.findOne({ + // Fallback to authentication provider if the user has Slack sign-in. + // Scoped via AuthenticationProvider to the matching Slack workspace so a + // colliding providerId from another team/provider cannot resolve. + const authentication = await UserAuthentication.findOne({ + where: { + providerId: serviceUserId, + }, + order: [["createdAt", "DESC"]], include: [ { - where: { - providerId: serviceUserId, - }, - order: [["createdAt", "DESC"]], - model: UserAuthentication, - as: "authentications", + model: AuthenticationProvider, + as: "authenticationProvider", required: true, + where: { + name: "slack", + providerId: serviceTeamId, + }, }, { - model: Team, - as: "team", + model: User, + as: "user", required: true, + include: [ + { + model: Team, + as: "team", + required: true, + }, + ], }, ], }); - if (user) { - return user; + if (authentication?.user) { + return authentication.user; } return;