fix(slack): verify BYO bot + app tokens are from the same app, and the app token is live (MUL-3666)

Niko review: RegisterBYO only parsed the app id from the xapp string and
auth.test'd the bot token, so pasting app A's bot token with app B's app
token would 'connect' but be broken (inbound on B's socket, outbound with
A's identity). Now: resolve the bot's owning app id via bots.info (on the
bot_id from auth.test) and require it to equal the xapp's app id; and live-
validate the app token via apps.connections.open. Reject (no persist) on
mismatch or a dead app token.

Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
J
2026-06-26 19:47:34 +08:00
parent 88c3ab7462
commit 889795125d
3 changed files with 141 additions and 16 deletions

View File

@@ -143,7 +143,7 @@ func (h *Handler) RegisterSlackBYO(w http.ResponseWriter, r *http.Request) {
})
if err != nil {
switch {
case errors.Is(err, slack.ErrInvalidBotToken), errors.Is(err, slack.ErrInvalidAppToken):
case errors.Is(err, slack.ErrInvalidBotToken), errors.Is(err, slack.ErrInvalidAppToken), errors.Is(err, slack.ErrTokenAppMismatch):
writeError(w, http.StatusBadRequest, err.Error())
case errors.Is(err, slack.ErrTeamOwnedByAnotherWorkspace):
writeError(w, http.StatusConflict, "this Slack app is already connected to a different Multica workspace")

View File

@@ -22,6 +22,11 @@ import (
var (
ErrInvalidBotToken = errors.New("slack: bot token must start with xoxb-")
ErrInvalidAppToken = errors.New("slack: app-level token must start with xapp- and embed an app id")
// ErrTokenAppMismatch is returned when the pasted bot token and app-level
// token belong to DIFFERENT Slack apps. Persisting that pair would "connect"
// but be broken: inbound arrives on the app token's socket (routed by its
// app id) while mention detection + outbound use the bot token's identity.
ErrTokenAppMismatch = errors.New("slack: the bot token and app-level token are from different Slack apps")
)
// RegisterBYOParams are the inputs for a bring-your-own-app install: the agent
@@ -69,8 +74,28 @@ func (s *InstallService) RegisterBYO(ctx context.Context, p RegisterBYOParams) (
if err != nil {
return db.ChannelInstallation{}, fmt.Errorf("slack auth.test: %w", err)
}
if auth.TeamID == "" || auth.UserID == "" {
return db.ChannelInstallation{}, errors.New("slack auth.test: response missing team_id / user_id")
if auth.TeamID == "" || auth.UserID == "" || auth.BotID == "" {
return db.ChannelInstallation{}, errors.New("slack auth.test: response missing team_id / user_id / bot_id")
}
// Prove the two tokens belong to the SAME Slack app: resolve the bot's
// OWNING app id (bots.info on the bot id auth.test returned) and require it to
// equal the app id embedded in the app-level token. Without this, pasting app
// A's bot token with app B's app token would "connect" but be broken —
// inbound arrives on app B's socket (routed by api_app_id=B) while mention
// detection + outbound use app A's bot identity / token (Niko review).
botAppID, err := s.botAppID(ctx, botToken, auth.BotID)
if err != nil {
return db.ChannelInstallation{}, fmt.Errorf("slack bots.info: %w", err)
}
if botAppID != appID {
return db.ChannelInstallation{}, ErrTokenAppMismatch
}
// Validate the app-level token is live (Socket Mode can actually open) so we
// never persist a token that will silently never receive events.
if err := s.validateAppToken(ctx, appToken); err != nil {
return db.ChannelInstallation{}, fmt.Errorf("slack apps.connections.open: %w", err)
}
sealedBot, err := s.box.Seal([]byte(botToken))
@@ -106,11 +131,11 @@ func (s *InstallService) RegisterBYO(ctx context.Context, p RegisterBYOParams) (
})
}
// authTest calls Slack auth.test with the given bot token, honoring the apiURL
// override so tests can point it at an httptest server (mirrors exchangeCode).
// The Slack SDK appends the method name to the endpoint, so the base must end
// in a slash.
func (s *InstallService) authTest(ctx context.Context, botToken string) (*slack.AuthTestResponse, error) {
// slackOpts builds the slack.Client options shared by the install-time Web API
// calls, honoring the apiURL override so tests can point them at an httptest
// server. The Slack SDK appends the method name to the endpoint, so the base
// must end in a slash. A fresh slice is returned each call (safe to append to).
func (s *InstallService) slackOpts() []slack.Option {
httpClient := s.httpClient
if httpClient == nil {
httpClient = http.DefaultClient
@@ -123,7 +148,33 @@ func (s *InstallService) authTest(ctx context.Context, botToken string) (*slack.
}
opts = append(opts, slack.OptionAPIURL(base))
}
return slack.New(botToken, opts...).AuthTestContext(ctx)
return opts
}
// authTest calls Slack auth.test with the bot token: validates it and returns
// the team id, the bot's own user id, and the bot id (for the bots.info lookup).
func (s *InstallService) authTest(ctx context.Context, botToken string) (*slack.AuthTestResponse, error) {
return slack.New(botToken, s.slackOpts()...).AuthTestContext(ctx)
}
// botAppID resolves the Slack app that OWNS the bot, via bots.info on the bot id
// from auth.test. It is the only token→app_id path for a bot token, so it is how
// we prove the pasted bot + app tokens belong to the same app.
func (s *InstallService) botAppID(ctx context.Context, botToken, botID string) (string, error) {
bot, err := slack.New(botToken, s.slackOpts()...).GetBotInfoContext(ctx, slack.GetBotInfoParameters{Bot: botID})
if err != nil {
return "", err
}
return bot.AppID, nil
}
// validateAppToken confirms the app-level token can open a Socket Mode
// connection (apps.connections.open) — a live check that the xapp is valid for
// THIS app, so we never store a token that will silently receive nothing.
func (s *InstallService) validateAppToken(ctx context.Context, appToken string) error {
api := slack.New("", append(s.slackOpts(), slack.OptionAppLevelToken(appToken))...)
_, _, err := api.StartSocketModeContext(ctx)
return err
}
// parseSlackAppID extracts the real Slack app id from an app-level token. The

View File

@@ -3,6 +3,7 @@ package slack
import (
"context"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"strings"
@@ -13,19 +14,51 @@ import (
db "github.com/multica-ai/multica/server/pkg/db/generated"
)
// authTestServer stubs Slack auth.test. ok=false drives the bad-token path.
func authTestServer(t *testing.T, ok bool) *httptest.Server {
// slackMock parameterizes the install-time Slack API stub. botAppID defaults to
// the app id embedded in byoParams' xapp token (so the same-app check passes).
type slackMock struct {
authOK bool // auth.test result
botAppID string // bots.info -> bot.app_id
appTokenOK bool // apps.connections.open result
}
// slackMockServer stubs the three Web API calls RegisterBYO makes: auth.test
// (bot token), bots.info (bot id -> owning app id), apps.connections.open (app
// token live check).
func slackMockServer(t *testing.T, m slackMock) *httptest.Server {
t.Helper()
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
if m.botAppID == "" {
m.botAppID = "A0BCXGVCS7R"
}
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
if !ok {
_, _ = w.Write([]byte(`{"ok":false,"error":"invalid_auth"}`))
return
switch r.URL.Path {
case "/auth.test":
if !m.authOK {
_, _ = w.Write([]byte(`{"ok":false,"error":"invalid_auth"}`))
return
}
_, _ = w.Write([]byte(`{"ok":true,"team_id":"T999","user_id":"UBOTBYO","bot_id":"B0BOT","team":"Acme Inc","url":"https://acme.slack.com/"}`))
case "/bots.info":
_, _ = w.Write([]byte(fmt.Sprintf(`{"ok":true,"bot":{"id":"B0BOT","app_id":%q,"user_id":"UBOTBYO"}}`, m.botAppID)))
case "/apps.connections.open":
if !m.appTokenOK {
_, _ = w.Write([]byte(`{"ok":false,"error":"invalid_auth"}`))
return
}
_, _ = w.Write([]byte(`{"ok":true,"url":"wss://example.test/link"}`))
default:
_, _ = w.Write([]byte(`{"ok":false,"error":"unknown_method"}`))
}
_, _ = w.Write([]byte(`{"ok":true,"team_id":"T999","user_id":"UBOTBYO","team":"Acme Inc","url":"https://acme.slack.com/"}`))
}))
}
// authTestServer is the happy-path stub (valid bot token, matching app id, live
// app token) unless ok=false, which makes auth.test reject the bot token.
func authTestServer(t *testing.T, ok bool) *httptest.Server {
return slackMockServer(t, slackMock{authOK: ok, appTokenOK: true})
}
func byoParams(ws, agent string) RegisterBYOParams {
return RegisterBYOParams{
WorkspaceID: pgtypeUUID(ws),
@@ -219,3 +252,44 @@ func TestRegisterBYO_AgentMove_RetiresStaleSessionBindings(t *testing.T) {
t.Fatal("an agent change must retire the installation's chat-session bindings")
}
}
func TestRegisterBYO_TokenAppMismatch(t *testing.T) {
// The bot token belongs to a DIFFERENT app (bots.info -> A0OTHER) than the
// app id embedded in the xapp token (A0BCXGVCS7R) — must be rejected so we
// never persist a broken installation (Niko review).
srv := slackMockServer(t, slackMock{authOK: true, botAppID: "A0OTHERAPP", appTokenOK: true})
defer srv.Close()
q := &fakeInstallQueries{}
svc := newTestInstallService(t, q)
svc.apiURL = srv.URL + "/"
if _, err := svc.RegisterBYO(context.Background(), byoParams(
"11111111-1111-1111-1111-111111111111",
"22222222-2222-2222-2222-222222222222",
)); err != ErrTokenAppMismatch {
t.Fatalf("mismatched tokens = %v, want ErrTokenAppMismatch", err)
}
if q.upsertCalled {
t.Error("mismatched bot/app tokens must be rejected before the upsert")
}
}
func TestRegisterBYO_AppTokenNotLive(t *testing.T) {
// auth.test + same-app check pass, but apps.connections.open rejects the app
// token — we must not persist a token that will never receive events.
srv := slackMockServer(t, slackMock{authOK: true, appTokenOK: false})
defer srv.Close()
q := &fakeInstallQueries{}
svc := newTestInstallService(t, q)
svc.apiURL = srv.URL + "/"
if _, err := svc.RegisterBYO(context.Background(), byoParams(
"11111111-1111-1111-1111-111111111111",
"22222222-2222-2222-2222-222222222222",
)); err == nil {
t.Fatal("expected an error when the app-level token is not live")
}
if q.upsertCalled {
t.Error("an invalid app token must not persist an installation")
}
}