fix(botsfw)!: scope bot settings lookup by platform

GetBotContext accepted a platformID parameter and never read it. It resolved purely
by bot ID or code, so the platform a webhook arrived on had no bearing on which
bot's settings came back.

With Telegram as the only framework-side platform this was invisible. bots-fw-whatsapp
now exists, which is exactly what makes it reachable: a WhatsApp webhook could
resolve a Telegram bot's settings - including its Token and WebhookSecretToken -
whenever the two shared an ID or code. Note bots-fw-telegram's handler passes
PlatformID and then verifies its secret against whatever settings came back.

Two bugs, one root cause: bot codes were treated as globally unique when they are
only unique per platform.

1. GetBotContext is now platform-scoped, via BotSettingsBy.findByPlatform. A bot on
   another platform returns ErrUnknownBot rather than being silently substituted. A
   blank platformID is rejected.

2. NewBotSettingsBy panicked on duplicate codes regardless of platform, so the same
   product could not be registered on two platforms - "debtus" on Telegram and
   "debtus" on WhatsApp are different bots with different tokens. It now panics only
   on a duplicate WITHIN a platform. This is what made a second platform registrable
   at all.

Added BotSettingsBy.ByPlatformAndCode / ByPlatformAndID. ByCode and ByID are kept
populated (first-wins) for compatibility but are now deprecated: they are ambiguous
across platforms by construction. The legacy fallback in findByPlatform still checks
Platform, so it cannot reintroduce the leak it exists to be compatible with.

The test suite had ENCODED the bug: three separate fixtures hand-built a BotSettings
with no Platform and passed only because platformID was ignored. All three are fixed
rather than worked around. NewBotSettings has always panicked on an empty platform,
so those fixtures were malformed - they bypassed the constructor.

BREAKING:
- A BotSettings with an empty Platform now panics in NewBotSettingsBy. Previously
  accepted, but such a bot can never be resolved by the platform-scoped
  GetBotContext, so it was silently unreachable. Failing at registration beats
  failing at 3am.
- GetBotContext returns ErrUnknownBot for a bot that exists on a different platform.
  That is the fix, not a regression.

Verified: bots-fw-telegram builds and its tests pass against this branch (temporary
local replace, reverted; that repo is untouched). Full suite green, gofmt + vet +
golangci-lint clean.

Background: spec/research/bots-fw-platform-neutrality-gap-analysis.md "Seam bug" in
bots-go-framework/backstage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: trakhimenok

botsfw/bot_context.go

@@ -44,13 +44,26 @@ func NewBotContextProvider(botHost BotHost, appContext AppContext, botSettingPro
 
 var ErrUnknownBot = errors.New("unknown bot")
 
+// GetBotContext returns the BotContext for a bot on a specific platform.
+//
+// The lookup is scoped to platformID. This matters: before it was, this method
+// accepted platformID and ignored it, resolving purely by bot ID or code. With a
+// single platform that was invisible. With two, a webhook for platform A could
+// resolve platform B's settings — including its Token and WebhookSecretToken —
+// whenever the two shared an ID or code.
+//
+// Returns ErrUnknownBot when no bot with that ID or code exists ON THAT PLATFORM,
+// even if one exists elsewhere. That is deliberate: a caller asking for a WhatsApp
+// bot must never be handed a Telegram one.
 func (v botContextProvider) GetBotContext(ctx context.Context, platformID botsfwconst.Platform, botID string) (botContext *BotContext, err error) {
+	if platformID == "" {
+		return nil, errors.New("required parameter platformID is empty")
+	}
 	botSettingsBy := v.botSettingProvider(ctx)
-	botSettings, ok := botSettingsBy.ByID[botID]
-	if !ok {
-		if botSettings, ok = botSettingsBy.ByCode[botID]; !ok {
-			return nil, ErrUnknownBot
-		}
+
+	botSettings := botSettingsBy.findByPlatform(platformID, botID)
+	if botSettings == nil {
+		return nil, ErrUnknownBot
 	}
 	return &BotContext{
 		AppContext:  v.appContext,

botsfw/bot_context_test.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"testing"
 
+	"github.com/bots-go-framework/bots-fw/botsfwconst"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -53,7 +54,10 @@ func (p testBotSettingsProvider) provide(_ context.Context) BotSettingsBy {
 
 func TestNewBotContextProvider(t *testing.T) {
 	host := testBotHost{}
-	settings := &BotSettings{Code: "mybot"}
+	// Platform is required: GetBotContext is platform-scoped, and NewBotSettings
+	// panics without one. A BotSettings with a blank Platform is malformed and no
+	// longer resolves for any platform.
+	settings := &BotSettings{Code: "mybot", Platform: botsfwconst.PlatformTelegram}
 	settingsBy := BotSettingsBy{
 		ByCode: map[string]*BotSettings{"mybot": settings},
 		ByID:   map[string]*BotSettings{},
@@ -99,3 +103,88 @@ func TestNewBotContextProvider(t *testing.T) {
 		assert.ErrorIs(t, err, ErrUnknownBot)
 	})
 }
+
+// TestGetBotContext_isPlatformScoped is the security regression test.
+//
+// Before this was scoped, GetBotContext accepted platformID and ignored it,
+// resolving purely by ID or code. With Telegram as the only platform that was
+// invisible. With two, a WhatsApp webhook could resolve a Telegram bot's settings
+// — including its Token and WebhookSecretToken.
+func TestGetBotContext_isPlatformScoped(t *testing.T) {
+	tgBot := BotSettings{
+		Platform: botsfwconst.PlatformTelegram, Code: "debtus", ID: "debtus",
+		Token: "TELEGRAM-SECRET-TOKEN", Profile: newTestProfile("debtus-profile"),
+	}
+	waBot := BotSettings{
+		Platform: botsfwconst.PlatformWhatsApp, Code: "debtus", ID: "debtus",
+		Token: "WHATSAPP-SECRET-TOKEN", Profile: newTestProfile("debtus-profile"),
+	}
+
+	// The same product on two platforms shares a code. This must be registrable —
+	// it used to panic with "Bot with duplicate code".
+	settingsBy := NewBotSettingsBy(tgBot, waBot)
+	provider := testBotSettingsProvider{settingsBy: settingsBy}
+	bcp := NewBotContextProvider(testBotHost{}, &testAppContext{}, provider.provide)
+	ctx := context.Background()
+
+	t.Run("each platform resolves its own bot", func(t *testing.T) {
+		tg, err := bcp.GetBotContext(ctx, botsfwconst.PlatformTelegram, "debtus")
+		assert.NoError(t, err)
+		assert.Equal(t, "TELEGRAM-SECRET-TOKEN", tg.BotSettings.Token)
+
+		wa, err := bcp.GetBotContext(ctx, botsfwconst.PlatformWhatsApp, "debtus")
+		assert.NoError(t, err)
+		assert.Equal(t, "WHATSAPP-SECRET-TOKEN", wa.BotSettings.Token,
+			"a WhatsApp webhook must never be handed Telegram's token")
+	})
+
+	t.Run("a bot on another platform is unknown, not silently substituted", func(t *testing.T) {
+		_, err := bcp.GetBotContext(ctx, botsfwconst.Platform("viber"), "debtus")
+		assert.ErrorIs(t, err, ErrUnknownBot)
+	})
+
+	t.Run("blank platform is rejected", func(t *testing.T) {
+		_, err := bcp.GetBotContext(ctx, "", "debtus")
+		assert.Error(t, err)
+	})
+}
+
+// TestGetBotContext_legacyFlatMapsStillCheckPlatform pins that the compatibility
+// fallback for a hand-built BotSettingsBy cannot reintroduce the leak it exists to
+// be compatible with.
+func TestGetBotContext_legacyFlatMapsStillCheckPlatform(t *testing.T) {
+	tgOnly := &BotSettings{Platform: botsfwconst.PlatformTelegram, Code: "mybot"}
+	settingsBy := BotSettingsBy{ // built by hand, no platform maps
+		ByCode: map[string]*BotSettings{"mybot": tgOnly},
+		ByID:   map[string]*BotSettings{},
+	}
+	bcp := NewBotContextProvider(testBotHost{}, &testAppContext{},
+		testBotSettingsProvider{settingsBy: settingsBy}.provide)
+	ctx := context.Background()
+
+	bc, err := bcp.GetBotContext(ctx, botsfwconst.PlatformTelegram, "mybot")
+	assert.NoError(t, err, "the legacy fallback must still resolve its own platform")
+	assert.Equal(t, "mybot", bc.BotSettings.Code)
+
+	_, err = bcp.GetBotContext(ctx, botsfwconst.PlatformWhatsApp, "mybot")
+	assert.ErrorIs(t, err, ErrUnknownBot,
+		"the legacy fallback must NOT hand a Telegram bot to a WhatsApp webhook")
+}
+
+// TestNewBotSettingsBy_duplicateCodeAcrossPlatforms pins that codes are unique per
+// platform rather than globally — the change that makes a second platform
+// registrable at all.
+func TestNewBotSettingsBy_duplicateCodeAcrossPlatforms(t *testing.T) {
+	tg := BotSettings{Platform: botsfwconst.PlatformTelegram, Code: "debtus", Profile: newTestProfile("debtus-profile")}
+	wa := BotSettings{Platform: botsfwconst.PlatformWhatsApp, Code: "debtus", Profile: newTestProfile("debtus-profile")}
+
+	assert.NotPanics(t, func() { NewBotSettingsBy(tg, wa) },
+		"the same code on two platforms is legitimate")
+
+	assert.Panics(t, func() { NewBotSettingsBy(tg, tg) },
+		"a duplicate code WITHIN a platform is still a bug")
+
+	assert.Panics(t, func() {
+		NewBotSettingsBy(BotSettings{Code: "nope", Profile: newTestProfile("debtus-profile")})
+	}, "a bot with no platform is malformed")
+}

botsfw/settings.go

@@ -154,48 +154,121 @@ type BotSettingsProvider func(ctx context.Context) BotSettingsBy
 // TODO: Decide if it should have map[string]*BotSettings instead of map[string]BotSettings
 type BotSettingsBy struct {
 
-	// ByCode keeps settings by bot code - it is a human-readable ID of a bot
+	// ByCode keeps settings by bot code - it is a human-readable ID of a bot.
+	//
+	// Deprecated: ambiguous once more than one platform is in play, because the
+	// same code may legitimately be used for the same product on Telegram and on
+	// WhatsApp. First registration wins here. Use ByPlatformAndCode, or resolve
+	// via BotContextProvider.GetBotContext which is platform-scoped.
 	ByCode map[string]*BotSettings
 
 	// ByID keeps settings by bot ID - it is a machine-readable ID of a bot.
+	//
+	// Deprecated: ambiguous across platforms, as ByCode. Use ByPlatformAndID.
 	ByID map[string]*BotSettings
 
 	ByProfile map[string][]*BotSettings
+
+	// ByPlatformAndCode keeps settings by platform, then bot code.
+	//
+	// Bot codes are only unique WITHIN a platform: "debtus" on Telegram and
+	// "debtus" on WhatsApp are different bots with different tokens.
+	ByPlatformAndCode map[botsfwconst.Platform]map[string]*BotSettings
+
+	// ByPlatformAndID keeps settings by platform, then bot ID.
+	ByPlatformAndID map[botsfwconst.Platform]map[string]*BotSettings
+}
+
+// findByPlatform resolves a bot by ID or code within a single platform.
+//
+// Returns nil if no such bot exists on that platform, even if one exists on
+// another — which is the point.
+func (settingsBy BotSettingsBy) findByPlatform(platform botsfwconst.Platform, botID string) *BotSettings {
+	if byID, ok := settingsBy.ByPlatformAndID[platform]; ok {
+		if s, ok := byID[botID]; ok {
+			return s
+		}
+	}
+	if byCode, ok := settingsBy.ByPlatformAndCode[platform]; ok {
+		if s, ok := byCode[botID]; ok {
+			return s
+		}
+	}
+	// Fall back to the legacy flat maps for a BotSettingsBy built by hand rather
+	// than by NewBotSettingsBy — but still verify the platform, so the fallback
+	// cannot reintroduce the cross-platform leak it exists to be compatible with.
+	if s, ok := settingsBy.ByID[botID]; ok && s.Platform == platform {
+		return s
+	}
+	if s, ok := settingsBy.ByCode[botID]; ok && s.Platform == platform {
+		return s
+	}
+	return nil
 }
 
 // NewBotSettingsBy create settings per different keys (ID, code, API token, Locale)
+//
+// Bot codes and IDs must be unique WITHIN a platform, not globally. The same
+// product on two platforms legitimately shares a code — "debtus" on Telegram and
+// "debtus" on WhatsApp are different bots with different tokens — and rejecting
+// that made a second platform impossible to register.
 func NewBotSettingsBy(bots ...BotSettings) (settingsBy BotSettingsBy) {
 	count := len(bots)
 	if count == 0 {
 		panic("NewBotSettingsBy: missing required parameter: bots")
 	}
 	settingsBy = BotSettingsBy{
-		ByCode:    make(map[string]*BotSettings, count),
-		ByID:      make(map[string]*BotSettings, count),
-		ByProfile: make(map[string][]*BotSettings),
+		ByCode:            make(map[string]*BotSettings, count),
+		ByID:              make(map[string]*BotSettings, count),
+		ByProfile:         make(map[string][]*BotSettings),
+		ByPlatformAndCode: make(map[botsfwconst.Platform]map[string]*BotSettings),
+		ByPlatformAndID:   make(map[botsfwconst.Platform]map[string]*BotSettings),
 	}
 	processBotSettings := func(i int, bot BotSettings) {
 		if bot.Code == "" {
 			panic(fmt.Sprintf("Bot with empty code at index %v", i))
 		}
-		if _, ok := settingsBy.ByCode[bot.Code]; ok {
-			panic(fmt.Sprintf("Bot with duplicate code: %v", bot.Code))
-		} else {
+		if bot.Platform == "" {
+			panic(fmt.Sprintf("Bot with empty platform at index %v, code=%v", i, bot.Code))
+		}
+
+		byCode, ok := settingsBy.ByPlatformAndCode[bot.Platform]
+		if !ok {
+			byCode = make(map[string]*BotSettings, count)
+			settingsBy.ByPlatformAndCode[bot.Platform] = byCode
+		}
+		if _, ok = byCode[bot.Code]; ok {
+			panic(fmt.Sprintf("Bot with duplicate code on platform %v: %v", bot.Platform, bot.Code))
+		}
+		byCode[bot.Code] = &bot
+
+		if bot.ID != "" {
+			byID, ok := settingsBy.ByPlatformAndID[bot.Platform]
+			if !ok {
+				byID = make(map[string]*BotSettings, count)
+				settingsBy.ByPlatformAndID[bot.Platform] = byID
+			}
+			if _, ok = byID[bot.ID]; ok {
+				panic(fmt.Sprintf("Bot with duplicate ID on platform %v: %v", bot.Platform, bot.ID))
+			}
+			byID[bot.ID] = &bot
+		}
+
+		// The legacy flat maps are kept populated for compatibility, first
+		// registration wins. They are ambiguous across platforms by construction,
+		// which is why they are deprecated and why GetBotContext no longer trusts
+		// them without checking Platform.
+		if _, ok = settingsBy.ByCode[bot.Code]; !ok {
 			settingsBy.ByCode[bot.Code] = &bot
 		}
 		if bot.ID != "" {
-			if _, ok := settingsBy.ByID[bot.ID]; ok {
-				panic(fmt.Sprintf("Bot with duplicate ID: %v", bot.ID))
-			} else {
+			if _, ok = settingsBy.ByID[bot.ID]; !ok {
 				settingsBy.ByID[bot.ID] = &bot
 			}
 		}
+
 		profileID := bot.Profile.ID()
-		if profileBots, ok := settingsBy.ByProfile[profileID]; ok {
-			settingsBy.ByProfile[profileID] = append(profileBots, &bot)
-		} else {
-			settingsBy.ByProfile[profileID] = []*BotSettings{&bot}
-		}
+		settingsBy.ByProfile[profileID] = append(settingsBy.ByProfile[profileID], &bot)
 	}
 	for i, bot := range bots {
 		processBotSettings(i, bot)

botswebhook/settings_test.go

@@ -90,18 +90,34 @@ func TestNewBotSettingsBy(t *testing.T) {
 			expectsPanic: true,
 		},
 		{
+			// Platform is required: bot codes are unique per platform, and a bot
+			// with no platform can never be resolved by the platform-scoped
+			// GetBotContext, so registering one is always a mistake.
 			name: "single_bot",
 			args: args{
 				bots: []botsfw.BotSettings{
 					{
-						Profile: testBotProfile,
-						Code:    "TestBot",
-						ID:      "test123",
+						Platform: botsfwconst.PlatformTelegram,
+						Profile:  testBotProfile,
+						Code:     "TestBot",
+						ID:       "test123",
 					},
 				},
 			},
 			expectsPanic: false,
 		},
+		{
+			name: "bot_without_platform",
+			args: args{
+				bots: []botsfw.BotSettings{
+					{
+						Profile: testBotProfile,
+						Code:    "NoPlatformBot",
+					},
+				},
+			},
+			expectsPanic: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -113,7 +129,13 @@ func TestNewBotSettingsBy(t *testing.T) {
 				}()
 			}
 			actual := botsfw.NewBotSettingsBy(tt.args.bots...)
-			assert.Equal(t, len(tt.args.bots), len(actual.ByCode))
+			// Count via the platform-scoped map: ByCode is deprecated and
+			// first-wins, so it under-counts when a code is shared across platforms.
+			var registered int
+			for _, byCode := range actual.ByPlatformAndCode {
+				registered += len(byCode)
+			}
+			assert.Equal(t, len(tt.args.bots), registered)
 		})
 	}
 }