From 303d091c43b09714170e22de1284d700e7f27a6f Mon Sep 17 00:00:00 2001 From: Sergey Lavrinenko Date: Tue, 8 Sep 2026 18:21:32 +0300 Subject: [PATCH] feat: chat scope for API keys (team isolation on /send, /alertmanager, /grafana) --- docs/integrations.md | 64 ++++ internal/cmd/serve.go | 83 ++++- internal/cmd/serve_scope_test.go | 95 ++++++ internal/cmd/server_apikey.go | 28 +- internal/config/config.go | 44 ++- internal/config/config_scope_test.go | 44 +++ internal/server/api/openapi.yaml | 10 +- internal/server/auth.go | 154 ++++++++- internal/server/handler_alertmanager.go | 6 +- internal/server/handler_config.go | 12 +- internal/server/handler_grafana.go | 6 +- internal/server/handler_send.go | 23 +- internal/server/key_scope_test.go | 409 ++++++++++++++++++++++++ internal/server/server.go | 29 +- 14 files changed, 976 insertions(+), 31 deletions(-) create mode 100644 internal/cmd/serve_scope_test.go create mode 100644 internal/config/config_scope_test.go create mode 100644 internal/server/key_scope_test.go diff --git a/docs/integrations.md b/docs/integrations.md index 8d4f5fd..5f00311 100644 --- a/docs/integrations.md +++ b/docs/integrations.md @@ -524,6 +524,70 @@ POST /api/v1/gitlab?chat_id= X-Gitlab-Token: → 4 строки в конфиге (один литерал или одна и та же `env:`/`vault:` ссылка дважды) ловит уже `config validate`. +### Изоляция команд: скоуп api-ключа (`/send`, `/alertmanager`, `/grafana`) + +Senders изолируют gitlab-ручку, где адресата выбирает не клиент. На остальных +ручках адресата присылает вызывающий, поэтому изоляция там устроена иначе: +api-ключу задаётся **белый список чатов**, и запрос в чат вне списка отклоняется. + +```yaml +server: + api_keys: + - name: alertmanager + key: env:ALERTMANAGER_KEY # без chats — доступ ко всем чатам + - name: b2c-at + key: env:B2C_AT_KEY + chats: [b2c-app-at] # только этот чат +``` + +Правила: + +- Ключ **без** `chats` не ограничен — конфиги, написанные до появления скоупа, + работают как раньше. +- Проверяется **финальный адресат**, после резолва алиаса и после подстановки + `default_chat_id` / дефолтного чата. Обратиться к чужому чату по голому UUID + в обход алиаса нельзя — сравниваются UUID, а не строки запроса. +- Отказ — `403 chat not allowed for this key`, без подсказок о том, какие чаты + существуют. Скоупленному ключу `GET /chats/alias/list` показывает только его + чаты; несуществующий чат и чужой чат отвечают **одинаково**, иначе по разнице + ответов перебираются имена чужих алиасов. Ключ без скоупа сохраняет подробную + диагностику — от него всё равно ничего не скрыто. +- Один и тот же секрет в конфиге и в `--api-key`/`EXPRESS_BOTX_SERVER_API_KEY` — + **ошибка на старте**: последние заданы без скоупа, а сервер индексирует ключи по + значению, поэтому безскоупная запись молча заменила бы скоупленную. Это + единственное ужесточение для существующих конфигов: раньше такая пара молча + работала, теперь сервер не поднимется. Повторяющееся `name` при разных значениях + безопасно (скоуп привязан к креду, а не к имени) — только предупреждение в лог. +- При **фанауте** (`chat_id=a,b,c`) скоуп применяется к каждому адресату списка. + В sync-режиме адреса резолвятся заранее, поэтому список, где хотя бы один чат + вне скоупа, отклоняется целиком и не доставляется никуда — частичной отправки + не бывает. В async адреса резолвит каталог внутри конвейера: там отказ приходит + по каждому адресату отдельно, и если отказано по всем — ответ `403`, а не `502`. +- `chats: []` — ошибка валидации: пустой список почти всегда опечатка, а тихий + запрет всего неотличим от поломки. Нужен неограниченный ключ — не пишите поле. +- Аутентификация bot-секретом (`allow_bot_secret_auth`) ключу конфига не + соответствует и скоупа не имеет — включайте её, только если это приемлемо. +- В async-режиме (`--enqueue`) адресат резолвится по **каталогу ботов**, который + может расходиться с локальным конфигом: один и тот же алиас там способен + указывать на другой чат. Поэтому скоуп там применяется в момент, когда адрес + становится окончательным, — непосредственно перед публикацией в очередь. + Свой `sendFn` (встраивание пакета `server` в чужой код) обязан вызвать + `server.ChatAllowed(ctx, chatID)` после резолва и вернуть + `server.ErrChatNotAllowed`, иначе скоуп в async-режиме не сработает. +- Значения чатов — алиасы из секции `chats` или UUID; резолв делается один раз + на старте, неизвестный алиас ловится `config validate`. + +Отличие от senders в одной строке: **senders подменяют адресацию** (событие +уходит строго в чаты sender'а), **скоуп api-ключа фильтрует** ту, что прислал +клиент. + +Через CLI: + +```sh +express-botx config apikey add --name b2c-at --chat b2c-app-at +express-botx config apikey list # покажет скоуп каждого ключа +``` + В GitLab каждая команда настраивает свой webhook как обычно ([Настройка GitLab](#настройка-gitlab)), указывая в **Secret token** значение своего sender-токена. diff --git a/internal/cmd/serve.go b/internal/cmd/serve.go index e94cc5d..5577ff6 100644 --- a/internal/cmd/serve.go +++ b/internal/cmd/serve.go @@ -12,6 +12,7 @@ import ( "os/signal" "path/filepath" "strconv" + "strings" "syscall" "time" @@ -186,7 +187,7 @@ Options: } // Resolve API keys - keys, err := resolveAPIKeys(cfg.Server.APIKeys) + keys, err := resolveAPIKeys(cfg.Server.APIKeys, cfg.Chats) if err != nil { return fmt.Errorf("resolving api keys: %w", err) } @@ -209,6 +210,10 @@ Options: keys = append(keys, server.ResolvedKey{Name: "env", Key: resolved}) } + if err := checkKeyCollisions(keys); err != nil { + return err + } + // Bot secret auth (only for bots with secret, not token-only) if cfg.Server.AllowBotSecretAuth { srvCfg.BotSignatures = make(map[string]string) @@ -434,18 +439,74 @@ Options: return srv.Run(ctx) } -func resolveAPIKeys(keys []config.APIKeyConfig) ([]server.ResolvedKey, error) { +func resolveAPIKeys(keys []config.APIKeyConfig, chats map[string]config.ChatConfig) ([]server.ResolvedKey, error) { resolved := make([]server.ResolvedKey, 0, len(keys)) + seenNames := make(map[string]bool, len(keys)) for _, k := range keys { + if seenNames[k.Name] { + // Names no longer index anything — a key's scope travels with the + // credential — so a repeat is confusing rather than dangerous, and + // refusing to start over it would break deployments that have been + // running happily. The colliding values below are the real hazard. + vlog.Info("config: duplicate API key name %q: log lines and errors will not tell these keys apart", k.Name) + } + seenNames[k.Name] = true val, err := secret.Resolve(k.Key) if err != nil { return nil, fmt.Errorf("key %q: %w", k.Name, err) } - resolved = append(resolved, server.ResolvedKey{Name: k.Name, Key: val}) + scope, err := resolveKeyScope(k, chats) + if err != nil { + return nil, err + } + resolved = append(resolved, server.ResolvedKey{Name: k.Name, Key: val, Chats: scope}) } return resolved, nil } +// checkKeyCollisions rejects two credentials that resolve to the same value. +// It runs over the assembled set rather than over the configured keys alone: +// --api-key and EXPRESS_BOTX_SERVER_API_KEY are appended afterwards and carry +// no scope, and the server indexes keys by value, so a collision there would +// not be ambiguous but silently authoritative — the unscoped entry would take +// the place of the scoped one and hand its holder unrestricted access. +func checkKeyCollisions(keys []server.ResolvedKey) error { + seen := make(map[string]string, len(keys)) + for _, k := range keys { + if owner, dup := seen[k.Key]; dup { + return fmt.Errorf("API keys %q and %q resolve to the same value: one would silently replace the other's chat scope", owner, k.Name) + } + seen[k.Key] = k.Name + } + return nil +} + +// resolveKeyScope turns a key's configured chats (aliases or UUIDs) into the +// UUID list compared at request time. Resolving here, once, is what keeps the +// check honest: a scope kept as aliases would not match a request addressing +// the same chat by its bare UUID. +func resolveKeyScope(k config.APIKeyConfig, chats map[string]config.ChatConfig) ([]string, error) { + if k.Chats == nil { + return nil, nil + } + if len(k.Chats) == 0 { + return nil, fmt.Errorf("key %q: chats must not be empty; omit the field to leave the key unrestricted", k.Name) + } + scope := make([]string, 0, len(k.Chats)) + for _, c := range k.Chats { + // Resolved by the very function delivery uses, so a reference cannot + // mean one chat when a scope is built and another when a message is + // addressed. Ordering these checks by hand is what let an alias whose + // name is itself a UUID resolve differently on the two paths. + id, _, err := config.ResolveChatRef(chats, c) + if err != nil { + return nil, fmt.Errorf("key %q: %w", k.Name, err) + } + scope = append(scope, strings.ToLower(id)) + } + return scope, nil +} + func buildSendRequest(p *server.SendPayload) *botapi.SendRequest { params := &botapi.SendParams{ ChatID: p.ChatID, @@ -903,7 +964,7 @@ func runServeEnqueue(flags config.Flags, listenFlag, apiKeyFlag, tlsCertFlag, tl srvCfg.AppVersion = Version // Resolve API keys - keys, err := resolveAPIKeys(cfg.Server.APIKeys) + keys, err := resolveAPIKeys(cfg.Server.APIKeys, cfg.Chats) if err != nil { return fmt.Errorf("resolving api keys: %w", err) } @@ -924,6 +985,10 @@ func runServeEnqueue(flags config.Flags, listenFlag, apiKeyFlag, tlsCertFlag, tl keys = append(keys, server.ResolvedKey{Name: "env", Key: resolved}) } + if err := checkKeyCollisions(keys); err != nil { + return err + } + if len(keys) == 0 { key, err := generateAPIKey() if err != nil { @@ -1020,7 +1085,7 @@ func runServeEnqueue(flags config.Flags, listenFlag, apiKeyFlag, tlsCertFlag, tl if chatID != "" && !config.IsUUID(chatID) { chat, err := snap.ResolveChat(chatID) if err != nil { - return "", err + return "", fmt.Errorf("%w: %v", server.ErrChatUnresolved, err) } routeChatAlias = chatID chatID = chat.ID @@ -1064,6 +1129,14 @@ func runServeEnqueue(flags config.Flags, listenFlag, apiKeyFlag, tlsCertFlag, tl return "", fmt.Errorf("chat_id is required") } + // chatID is final here: catalog resolution has run, and nothing between + // this point and delivery changes it. Applying the key's chat scope + // anywhere earlier would authorize the address the local config names + // while the catalog sends the message somewhere else. + if !server.ChatAllowed(ctx, chatID) { + return "", server.ErrChatNotAllowed + } + msg := &queue.WorkMessage{ RequestID: requestID, Routing: queue.Routing{ diff --git a/internal/cmd/serve_scope_test.go b/internal/cmd/serve_scope_test.go new file mode 100644 index 0000000..526d027 --- /dev/null +++ b/internal/cmd/serve_scope_test.go @@ -0,0 +1,95 @@ +package cmd + +import ( + "testing" + + "github.com/lavr/express-botx/internal/config" + "github.com/lavr/express-botx/internal/server" +) + +// serve and serve --enqueue never call Config.Validate, so an explicitly empty +// scope has to be rejected where keys are resolved. Falling back to "no scope" +// there would silently hand the key full access. +func TestResolveAPIKeys_EmptyScopeRejected(t *testing.T) { + chats := map[string]config.ChatConfig{"known": {ID: "bcb715a2-e8d3-57a8-ab3b-6a14c044dd22"}} + + if _, err := resolveAPIKeys([]config.APIKeyConfig{{Name: "k", Key: "v", Chats: []string{}}}, chats); err == nil { + t.Error("empty chats list accepted; the key would be unrestricted") + } + + keys, err := resolveAPIKeys([]config.APIKeyConfig{{Name: "k", Key: "v"}}, chats) + if err != nil { + t.Fatalf("absent scope rejected: %v", err) + } + if len(keys[0].Chats) != 0 { + t.Errorf("absent scope produced %v, want unrestricted", keys[0].Chats) + } +} + +func TestResolveAPIKeys_AliasResolvedToUUID(t *testing.T) { + chats := map[string]config.ChatConfig{"known": {ID: "BCB715A2-E8D3-57A8-AB3B-6A14C044DD22"}} + keys, err := resolveAPIKeys([]config.APIKeyConfig{{Name: "k", Key: "v", Chats: []string{"known"}}}, chats) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if got := keys[0].Chats[0]; got != "bcb715a2-e8d3-57a8-ab3b-6a14c044dd22" { + t.Errorf("scope = %q, want the lowercased UUID behind the alias", got) + } +} + +func TestResolveAPIKeys_Duplicates(t *testing.T) { + chats := map[string]config.ChatConfig{} + + // A repeated name is only confusing — the scope travels with the credential, + // not with the name — so it warns and starts rather than breaking a + // deployment that has been running with it. + keys, err := resolveAPIKeys([]config.APIKeyConfig{ + {Name: "same", Key: "a"}, + {Name: "same", Key: "b"}, + }, chats) + if err != nil { + t.Errorf("duplicate names refused startup: %v", err) + } + if len(keys) != 2 { + t.Errorf("got %d keys, want both kept", len(keys)) + } + +} + +// Collision detection has to run over the assembled key set, not over the +// configured keys alone: --api-key and EXPRESS_BOTX_SERVER_API_KEY are appended +// afterwards without a scope, and the server indexes keys by value, so a +// collision would silently replace a scoped entry with an unrestricted one. +func TestCheckKeyCollisions(t *testing.T) { + scoped := server.ResolvedKey{Name: "b2c-at", Key: "shared", Chats: []string{"bcb715a2-e8d3-57a8-ab3b-6a14c044dd22"}} + + if err := checkKeyCollisions([]server.ResolvedKey{scoped, {Name: "env", Key: "shared"}}); err == nil { + t.Error("env key silently took over a scoped key's value") + } + if err := checkKeyCollisions([]server.ResolvedKey{scoped, {Name: "cli", Key: "shared"}}); err == nil { + t.Error("cli key silently took over a scoped key's value") + } + if err := checkKeyCollisions([]server.ResolvedKey{scoped, {Name: "env", Key: "distinct"}}); err != nil { + t.Errorf("distinct values rejected: %v", err) + } +} + +// A reference must mean the same chat when a scope is built and when a message +// is addressed. Delivery resolves a UUID before consulting the alias map, so a +// scope that consulted aliases first would bind to a different chat whenever an +// alias is itself named like a UUID. +func TestResolveKeyScope_UUIDBeatsAliasOfTheSameName(t *testing.T) { + const ( + a = "bcb715a2-e8d3-57a8-ab3b-6a14c044dd22" + b = "7ee8aaa9-c6cb-5ee6-8445-7d654819b285" + ) + chats := map[string]config.ChatConfig{a: {ID: b}} + + keys, err := resolveAPIKeys([]config.APIKeyConfig{{Name: "k", Key: "v", Chats: []string{a}}}, chats) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if got := keys[0].Chats[0]; got != a { + t.Errorf("scope = %q, want %q: the reference is a UUID and delivery reads it as one", got, a) + } +} diff --git a/internal/cmd/server_apikey.go b/internal/cmd/server_apikey.go index c97d612..55458bf 100644 --- a/internal/cmd/server_apikey.go +++ b/internal/cmd/server_apikey.go @@ -37,12 +37,13 @@ func runServerAPIKeyList(args []string, deps Deps) error { } type apiKeyInfo struct { - Name string `json:"name"` - Source string `json:"source"` + Name string `json:"name"` + Source string `json:"source"` + Chats []string `json:"chats,omitempty"` } info := make([]apiKeyInfo, len(cfg.Server.APIKeys)) for i, k := range cfg.Server.APIKeys { - info[i] = apiKeyInfo{Name: k.Name, Source: describeKeySource(k.Key)} + info[i] = apiKeyInfo{Name: k.Name, Source: describeKeySource(k.Key), Chats: k.Chats} } return printOutput(deps.Stdout, cfg.Format, func() { @@ -53,7 +54,11 @@ func runServerAPIKeyList(args []string, deps Deps) error { } fmt.Fprintf(deps.Stdout, "API keys (%d):\n", len(info)) for _, k := range info { - fmt.Fprintf(deps.Stdout, " %-20s %s\n", k.Name, k.Source) + scope := "any chat" + if len(k.Chats) > 0 { + scope = strings.Join(k.Chats, ", ") + } + fmt.Fprintf(deps.Stdout, " %-20s %-12s %s\n", k.Name, k.Source, scope) } }, info) } @@ -63,12 +68,14 @@ func runServerAPIKeyAdd(args []string, deps Deps) error { fs.SetOutput(deps.Stderr) var flags config.Flags var name, key string + var chats stringSlice fs.StringVar(&flags.ConfigPath, "config", "", "path to config file") fs.StringVar(&name, "name", "", "key name (required)") fs.StringVar(&key, "key", "", "key value (generated if omitted)") + fs.Var(&chats, "chat", "restrict the key to this chat (alias or UUID); repeatable") fs.Usage = func() { - fmt.Fprintf(deps.Stderr, "Usage: express-botx config apikey add --name NAME [--key VALUE] [options]\n\nAdd an API key to the server config.\nIf --key is omitted, a random key is generated.\n\nOptions:\n") + fmt.Fprintf(deps.Stderr, "Usage: express-botx config apikey add --name NAME [--key VALUE] [--chat ALIAS]... [options]\n\nAdd an API key to the server config.\nIf --key is omitted, a random key is generated.\nWithout --chat the key may address any chat.\n\nOptions:\n") fs.PrintDefaults() } @@ -102,9 +109,16 @@ func runServerAPIKeyAdd(args []string, deps Deps) error { fmt.Fprintf(deps.Stdout, "Generated key: %s\n", key) } + for _, c := range chats { + if _, ok := cfg.Chats[c]; !ok && !config.IsUUID(c) { + return fmt.Errorf("unknown chat %q: use a configured alias or a UUID", c) + } + } + cfg.Server.APIKeys = append(cfg.Server.APIKeys, config.APIKeyConfig{ - Name: name, - Key: key, + Name: name, + Key: key, + Chats: chats, }) if err := cfg.SaveConfig(); err != nil { diff --git a/internal/config/config.go b/internal/config/config.go index ee6c4bb..3c3836a 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -190,6 +190,10 @@ type GitlabEventsConfig struct { type APIKeyConfig struct { Name string `yaml:"name" json:"name"` Key string `yaml:"key" json:"key"` // literal, env:VAR, or vault:path#key + // Chats optionally restricts this key to a fixed set of chats (aliases or + // UUIDs). A key without Chats may address any chat, preserving the + // behaviour of configs written before scoping existed. + Chats []string `yaml:"chats,omitempty" json:"chats,omitempty"` } type BotConfig struct { @@ -1134,7 +1138,7 @@ var knownKeys = map[string]map[string]bool{ "type": true, "command": true, "url": true, "timeout": true, }, "server.api_keys.*": { - "name": true, "key": true, + "name": true, "key": true, "chats": true, }, "queue": { "driver": true, "url": true, "name": true, "reply_queue": true, "group": true, "max_file_size": true, @@ -1335,6 +1339,44 @@ func (c *Config) validateRequiredFields() []ValidationResult { } } + // An API key may be scoped to a set of chats. A scope naming a chat that + // does not exist would silently deny everything at request time, so it is + // rejected here instead. An explicitly empty list is treated the same way: + // it is far more likely to be an editing accident than a deliberate + // "deny all", and a silent deny-all looks like a broken network. + for i, k := range c.Server.APIKeys { + if k.Chats == nil { + continue + } + path := fmt.Sprintf("server.api_keys[%d].chats", i) + if len(k.Chats) == 0 { + results = append(results, ValidationResult{ + Level: ValidationError, + Path: path, + Message: "chats must not be empty; omit the field to leave the key unrestricted", + }) + continue + } + seen := map[string]bool{} + for _, name := range k.Chats { + if _, ok := c.Chats[name]; !ok && !IsUUID(name) { + results = append(results, ValidationResult{ + Level: ValidationError, + Path: path, + Message: fmt.Sprintf("unknown chat %q: use a configured alias or a UUID", name), + }) + } + if seen[name] { + results = append(results, ValidationResult{ + Level: ValidationWarning, + Path: path, + Message: fmt.Sprintf("duplicate chat %q", name), + }) + } + seen[name] = true + } + } + if g := c.Server.Gitlab; g != nil { results = append(results, validateGitlabSenders(g)...) } diff --git a/internal/config/config_scope_test.go b/internal/config/config_scope_test.go new file mode 100644 index 0000000..effd0a7 --- /dev/null +++ b/internal/config/config_scope_test.go @@ -0,0 +1,44 @@ +package config + +import ( + "strings" + "testing" +) + +func TestValidate_APIKeyChatScope(t *testing.T) { + base := func(chats []string) *Config { + return &Config{ + Bots: map[string]BotConfig{"b": {Host: "h", ID: "i", Secret: "s"}}, + Chats: map[string]ChatConfig{"known": {ID: "bcb715a2-e8d3-57a8-ab3b-6a14c044dd22"}}, + Server: ServerConfig{ + APIKeys: []APIKeyConfig{{Name: "k", Key: "v", Chats: chats}}, + }, + } + } + + tests := []struct { + name string + chats []string + wantErr bool + }{ + {"absent scope", nil, false}, + {"known alias", []string{"known"}, false}, + {"bare uuid", []string{"7ee8aaa9-c6cb-5ee6-8445-7d654819b285"}, false}, + {"empty list", []string{}, true}, + {"unknown alias", []string{"nope"}, true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + var hasErr bool + for _, r := range base(tc.chats).Validate(nil) { + if r.Level == ValidationError && strings.Contains(r.Path, "api_keys") { + hasErr = true + } + } + if hasErr != tc.wantErr { + t.Errorf("error = %v, want %v", hasErr, tc.wantErr) + } + }) + } +} diff --git a/internal/server/api/openapi.yaml b/internal/server/api/openapi.yaml index 8fb84b4..a93d4a6 100644 --- a/internal/server/api/openapi.yaml +++ b/internal/server/api/openapi.yaml @@ -79,7 +79,9 @@ paths: get: operationId: chatsAliasList summary: List chat aliases - description: Returns the list of chat aliases from the config file, including bot bindings. + description: > + Returns the list of chat aliases from the config file, including bot bindings. + A key restricted to a chat scope sees only the chats in its own scope. responses: "200": description: List of chat aliases @@ -292,7 +294,7 @@ paths: schema: $ref: "#/components/schemas/ErrorResponse" "403": - description: Invalid credentials + description: Invalid credentials, or the target chat is outside this API key's chat scope content: application/json: schema: @@ -398,7 +400,7 @@ paths: schema: $ref: "#/components/schemas/ErrorResponse" "403": - description: Invalid credentials + description: Invalid credentials, or the target chat is outside this API key's chat scope content: application/json: schema: @@ -602,7 +604,7 @@ paths: schema: $ref: "#/components/schemas/ErrorResponse" "403": - description: Invalid credentials + description: Invalid credentials, or the target chat is outside this API key's chat scope content: application/json: schema: diff --git a/internal/server/auth.go b/internal/server/auth.go index f965adc..322fb78 100644 --- a/internal/server/auth.go +++ b/internal/server/auth.go @@ -12,8 +12,9 @@ import ( type ctxKey int const ( - keyNameKey ctxKey = iota - authBotKey // bot name bound by X-Bot-Signature auth + keyNameKey ctxKey = iota + authBotKey // bot name bound by X-Bot-Signature auth + keyScopeKey // chat scope of the API key that authenticated the request ) // KeyName returns the API key name from the request context. @@ -33,13 +34,156 @@ func AuthBot(ctx context.Context) string { return "" } +// chatAllowed reports whether the API key that authenticated this request may +// address the given chat. resolvedUUID must be the final delivery target — +// after alias resolution and after any default or fallback substitution. +// Checking the value as it arrived in the request would let a bare UUID walk +// straight past the scope, which is the hole this exists to close. +// +// A key with no configured scope is unrestricted, as is a request authenticated +// by bot signature rather than by an API key. +func (s *Server) chatAllowed(ctx context.Context, resolvedUUID string) bool { + return ChatAllowed(ctx, resolvedUUID) +} + +// scopedKey reports whether this request's API key carries a chat scope. Used +// to withhold catalog details (chat lists, alias suggestions) from callers that +// are not allowed to see the whole installation. +func (s *Server) scopedKey(ctx context.Context) bool { + return Scoped(ctx) +} + +// Scoped reports whether the API key behind this request is restricted to a set +// of chats. Exported for send pipelines that resolve the final address +// themselves and must apply the scope at that point. +func Scoped(ctx context.Context) bool { + return len(keyScope(ctx)) > 0 +} + +// ChatAllowed reports whether the API key behind this request may address the +// given chat UUID. +// +// Async delivery resolves aliases against the bot catalog, which can disagree +// with the local configuration: the same alias may name a different chat there. +// A check made against the local list would therefore authorize one chat while +// the message went to another, so the send pipeline must call this with the +// UUID it is actually about to deliver to, immediately before delivering. +func ChatAllowed(ctx context.Context, resolvedUUID string) bool { + scope := keyScope(ctx) + if len(scope) == 0 { + return true + } + target := strings.ToLower(resolvedUUID) + for _, allowed := range scope { + if allowed == target { + return true + } + } + return false +} + +// keyScope returns the chat scope of the API key that authenticated this +// request. The scope travels with the request rather than being looked up by +// key name: names are not guaranteed unique, and a name-keyed lookup would +// hand one key the permissions of another that happens to share its name. +func keyScope(ctx context.Context) []string { + if v, ok := ctx.Value(keyScopeKey).([]string); ok { + return v + } + return nil +} + +// authorizeTargets applies the key's chat scope to every chat a fan-out request +// names, before any of them is delivered to. Checking inside the fan-out would +// not be enough: delivery is best-effort and per target, so an unauthorized +// chat listed alongside an authorized one would be refused while the rest of +// the message still went out. A request naming any chat outside the scope is +// refused whole, and nothing is sent. +// +// Only the sync path can resolve here: in async mode the chat resolver is a +// pass-through and the real address comes from the bot catalog later, so a +// non-UUID target is left to the send pipeline (see ChatAllowed). Returns false +// when it has already written the response. +func (s *Server) authorizeTargets(w http.ResponseWriter, r *http.Request, targets []string, resolve bool) bool { + if !s.scopedKey(r.Context()) { + return true + } + for _, target := range targets { + if !resolve { + if isUUID(target) && !ChatAllowed(r.Context(), target) { + writeError(w, http.StatusForbidden, ErrChatNotAllowed.Error()) + return false + } + continue + } + chat, err := s.chats(target) + if err != nil { + s.denyChat(w, r, err, "") + return false + } + if !ChatAllowed(r.Context(), chat.ChatID) { + writeError(w, http.StatusForbidden, ErrChatNotAllowed.Error()) + return false + } + } + return true +} + +// sanitizeErrors rewrites per-chat failures for a scoped caller. Fan-out is +// best-effort and reports each target's error verbatim: those strings quote the +// chat catalog and distinguish "no such chat" from "not your chat", which is +// exactly the pair a scoped key must not be able to tell apart. +func (s *Server) sanitizeErrors(ctx context.Context, errs []SendError) []SendError { + if !Scoped(ctx) || len(errs) == 0 { + return errs + } + out := make([]SendError, len(errs)) + for i, e := range errs { + out[i] = SendError{Chat: e.Chat, Error: ErrChatNotAllowed.Error()} + } + return out +} + +// denyChat writes the refusal a scoped caller gets for any chat it may not use. +// Unknown and forbidden are answered identically on purpose: distinguishable +// responses turn the endpoint into an oracle, and a scoped key could enumerate +// other teams' alias names by guessing and watching the status code. Callers +// without a scope keep the diagnostic distinction, since nothing is hidden from +// them anyway. +func (s *Server) denyChat(w http.ResponseWriter, r *http.Request, err error, stage string) { + if s.scopedKey(r.Context()) { + writeError(w, http.StatusForbidden, "chat not allowed for this key") + return + } + msg := err.Error() + if stage != "" { + msg = stage + ": " + msg + } + writeError(w, http.StatusBadRequest, msg) +} + +// deliveryError renders a delivery failure for the client. Errors raised while +// resolving and sending quote the chat catalog — the async path resolves +// against the bot catalog, whose "available" list names chats belonging to +// other teams — so a scoped key is told only that the step failed. The full +// error is logged either way. +func (s *Server) deliveryError(ctx context.Context, stage string, err error) string { + if s.scopedKey(ctx) { + return stage + } + return stage + ": " + err.Error() +} + func (s *Server) authMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { // 1. Try API key (Bearer or X-API-Key) if key := extractKey(r); key != "" { - if name, ok := s.keyMap[key]; ok { - vlog.V1("server: %s %s [key: %s]", r.Method, r.URL.Path, name) - ctx := context.WithValue(r.Context(), keyNameKey, name) + if rk, ok := s.keyMap[key]; ok { + vlog.V1("server: %s %s [key: %s]", r.Method, r.URL.Path, rk.Name) + ctx := context.WithValue(r.Context(), keyNameKey, rk.Name) + if len(rk.Chats) > 0 { + ctx = context.WithValue(ctx, keyScopeKey, rk.Chats) + } next.ServeHTTP(w, r.WithContext(ctx)) return } diff --git a/internal/server/handler_alertmanager.go b/internal/server/handler_alertmanager.go index 043c3e6..7caa2a5 100644 --- a/internal/server/handler_alertmanager.go +++ b/internal/server/handler_alertmanager.go @@ -98,6 +98,10 @@ func (s *Server) handleAlertmanager(w http.ResponseWriter, r *http.Request) { return } + if !s.authorizeTargets(w, r, targets, true) { + return + } + start := time.Now() results, errs := s.fanoutSend(r.Context(), targets, r.URL.Query().Get("bot"), message, status) elapsed := time.Since(start) @@ -108,7 +112,7 @@ func (s *Server) handleAlertmanager(w http.ResponseWriter, r *http.Request) { } else { vlog.V1("alertmanager: sent %s to %d/%d chats [key: %s] (%dms)", webhook.Status, len(results), len(targets), keyName, elapsed.Milliseconds()) } - writeMultiSend(w, results, errs, http.StatusOK) + writeMultiSend(w, results, s.sanitizeErrors(r.Context(), errs), http.StatusOK) } // singleChat returns the fallback delivery chat for alertmanager, following the diff --git a/internal/server/handler_config.go b/internal/server/handler_config.go index d8297c6..fb9f0f9 100644 --- a/internal/server/handler_config.go +++ b/internal/server/handler_config.go @@ -14,7 +14,17 @@ func (s *Server) handleBotList(w http.ResponseWriter, r *http.Request) { func (s *Server) handleChatsAliasList(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(s.chatEntries) + entries := s.chatEntries + if s.scopedKey(r.Context()) { + visible := make([]config.ChatEntry, 0, len(entries)) + for _, e := range entries { + if s.chatAllowed(r.Context(), e.ID) { + visible = append(visible, e) + } + } + entries = visible + } + json.NewEncoder(w).Encode(entries) } // WithConfigInfo sets the bot and chat entries for the config endpoints. diff --git a/internal/server/handler_grafana.go b/internal/server/handler_grafana.go index fcbd3d0..651f173 100644 --- a/internal/server/handler_grafana.go +++ b/internal/server/handler_grafana.go @@ -109,6 +109,10 @@ func (s *Server) handleGrafana(w http.ResponseWriter, r *http.Request) { return } + if !s.authorizeTargets(w, r, targets, true) { + return + } + start := time.Now() results, errs := s.fanoutSend(r.Context(), targets, r.URL.Query().Get("bot"), message, status) elapsed := time.Since(start) @@ -119,7 +123,7 @@ func (s *Server) handleGrafana(w http.ResponseWriter, r *http.Request) { } else { vlog.V1("grafana: sent %s to %d/%d chats [key: %s] (%dms)", webhook.Status, len(results), len(targets), keyName, elapsed.Milliseconds()) } - writeMultiSend(w, results, errs, http.StatusOK) + writeMultiSend(w, results, s.sanitizeErrors(r.Context(), errs), http.StatusOK) } // singleChat returns the fallback delivery chat for grafana, following the diff --git a/internal/server/handler_send.go b/internal/server/handler_send.go index 80a9116..5086250 100644 --- a/internal/server/handler_send.go +++ b/internal/server/handler_send.go @@ -133,6 +133,9 @@ func (s *Server) handleSend(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "chat_id is required") return } + if !s.authorizeTargets(w, r, targets, false) { + return + } // Async mode: for direct routing, bot_id is required. // For catalog/mixed modes, bot_id or bot alias can be used. @@ -168,12 +171,20 @@ func (s *Server) handleSend(w http.ResponseWriter, r *http.Request) { } start := time.Now() + // Aliases cannot be authorized before the fan-out — async resolves them + // against the bot catalog inside the pipeline — so a refusal surfaces as + // a per-target error. When every target was refused, the request was an + // authorization failure, not a delivery one, and says so. + denied := 0 results, errs := fanout(r.Context(), targets, func(ctx context.Context, chat string) (SendResult, error) { // Copy the shared payload and enqueue one message for this chat only. p := payload p.ChatID = chat requestID, err := s.send(ctx, &p) if err != nil { + if errors.Is(err, ErrChatNotAllowed) || (errors.Is(err, ErrChatUnresolved) && s.scopedKey(r.Context())) { + denied++ + } return SendResult{}, err } return SendResult{Chat: chat, RequestID: requestID, Queued: true}, nil @@ -181,12 +192,17 @@ func (s *Server) handleSend(w http.ResponseWriter, r *http.Request) { elapsed := time.Since(start) keyName := KeyName(r.Context()) + if len(results) == 0 && denied == len(targets) { + vlog.V1("server: %s %s [key: %s] -> 403 (%dms)", r.Method, r.URL.Path, keyName, elapsed.Milliseconds()) + writeError(w, http.StatusForbidden, ErrChatNotAllowed.Error()) + return + } if len(results) == 0 { vlog.V1("server: %s %s [key: %s] -> 502 (%dms)", r.Method, r.URL.Path, keyName, elapsed.Milliseconds()) } else { vlog.V1("server: %s %s [key: %s] -> 202 (%dms)", r.Method, r.URL.Path, keyName, elapsed.Milliseconds()) } - writeMultiSend(w, results, errs, http.StatusAccepted) + writeMultiSend(w, results, s.sanitizeErrors(r.Context(), errs), http.StatusAccepted) return } @@ -203,6 +219,9 @@ func (s *Server) handleSend(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "chat_id is required") return } + if !s.authorizeTargets(w, r, targets, true) { + return + } start := time.Now() results, errs := fanout(r.Context(), targets, func(ctx context.Context, chat string) (SendResult, error) { @@ -248,7 +267,7 @@ func (s *Server) handleSend(w http.ResponseWriter, r *http.Request) { } else { vlog.V1("server: %s %s [key: %s] -> 200 (%dms)", r.Method, r.URL.Path, keyName, elapsed.Milliseconds()) } - writeMultiSend(w, results, errs, http.StatusOK) + writeMultiSend(w, results, s.sanitizeErrors(r.Context(), errs), http.StatusOK) } // validateAsyncRouting checks a single target chat against the async routing-mode diff --git a/internal/server/key_scope_test.go b/internal/server/key_scope_test.go new file mode 100644 index 0000000..68f7b29 --- /dev/null +++ b/internal/server/key_scope_test.go @@ -0,0 +1,409 @@ +package server + +import ( + "context" + "encoding/json" + "fmt" + "net/http/httptest" + "strings" + "testing" + + "github.com/lavr/express-botx/internal/config" +) + +const ( + ownUUID = "bcb715a2-e8d3-57a8-ab3b-6a14c044dd22" + otherUUID = "7ee8aaa9-c6cb-5ee6-8445-7d654819b285" +) + +// newScopeServer builds a server whose resolver maps two aliases onto two +// chats, so a scoped key can be aimed at its own chat and at a foreign one by +// either name form. +func newScopeServer(keys []ResolvedKey, srvOpts ...Option) *Server { + aliases := map[string]string{"own-chat": ownUUID, "other-chat": otherUUID} + cfg := Config{Listen: ":0", BasePath: "/api/v1", Keys: keys} + sendFn := func(ctx context.Context, p *SendPayload) (string, error) { return "sync-id", nil } + chatResolver := func(chatID string) (ChatResolveResult, error) { + if id, ok := aliases[chatID]; ok { + return ChatResolveResult{ChatID: id}, nil + } + if chatID == ownUUID || chatID == otherUUID { + return ChatResolveResult{ChatID: chatID}, nil + } + return ChatResolveResult{}, fmt.Errorf("unknown chat alias %q, available: other-chat, own-chat", chatID) + } + return New(cfg, sendFn, chatResolver, srvOpts...) +} + +func sendAs(srv *Server, key, chatID string) *httptest.ResponseRecorder { + body := strings.NewReader(fmt.Sprintf(`{"chat_id":%q,"message":"hi"}`, chatID)) + return doRequest(srv, "POST", "/api/v1/send", body, map[string]string{ + "Content-Type": "application/json", + "Authorization": "Bearer " + key, + }) +} + +func TestKeyScope_Send(t *testing.T) { + unscoped := ResolvedKey{Name: "any-app", Key: "open"} + scoped := ResolvedKey{Name: "b2c-at", Key: "narrow", Chats: []string{ownUUID}} + + tests := []struct { + name string + key string + chatID string + want int + }{ + {"unscoped key, own alias", "open", "own-chat", 200}, + {"unscoped key, foreign alias", "open", "other-chat", 200}, + {"unscoped key, foreign uuid", "open", otherUUID, 200}, + {"scoped key, own alias", "narrow", "own-chat", 200}, + {"scoped key, own uuid", "narrow", ownUUID, 200}, + {"scoped key, foreign alias", "narrow", "other-chat", 403}, + {"scoped key, foreign uuid", "narrow", otherUUID, 403}, + } + + srv := newScopeServer([]ResolvedKey{unscoped, scoped}) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + w := sendAs(srv, tc.key, tc.chatID) + if w.Code != tc.want { + t.Errorf("status = %d, want %d (body: %s)", w.Code, tc.want, w.Body.String()) + } + }) + } +} + +// The scope is stored and compared as UUIDs even when configured as an alias; +// a scope kept as an alias would not match a request using the bare UUID. +func TestKeyScope_UUIDCaseInsensitive(t *testing.T) { + scoped := ResolvedKey{Name: "b2c-at", Key: "narrow", Chats: []string{strings.ToUpper(ownUUID)}} + srv := newScopeServer([]ResolvedKey{scoped}) + if w := sendAs(srv, "narrow", ownUUID); w.Code != 200 { + t.Errorf("status = %d, want 200 (body: %s)", w.Code, w.Body.String()) + } +} + +func TestKeyScope_NoAliasLeakInError(t *testing.T) { + scoped := ResolvedKey{Name: "b2c-at", Key: "narrow", Chats: []string{ownUUID}} + unscoped := ResolvedKey{Name: "any-app", Key: "open"} + srv := newScopeServer([]ResolvedKey{scoped, unscoped}) + + w := sendAs(srv, "narrow", "no-such-alias") + if strings.Contains(w.Body.String(), "own-chat") { + t.Errorf("scoped key was told the alias catalog: %s", w.Body.String()) + } + + w = sendAs(srv, "open", "no-such-alias") + if !strings.Contains(w.Body.String(), "own-chat") { + t.Errorf("unscoped key lost the helpful alias list: %s", w.Body.String()) + } +} + +// The async path resolves against the bot catalog, whose errors quote every +// alias it knows — chats of other teams included. A scoped key must not be +// handed that list through a delivery failure. +func TestKeyScope_NoCatalogLeakInAsyncError(t *testing.T) { + scoped := ResolvedKey{Name: "b2c-at", Key: "narrow", Chats: []string{ownUUID}} + unscoped := ResolvedKey{Name: "any-app", Key: "open"} + + sendFn := func(ctx context.Context, p *SendPayload) (string, error) { + return "", fmt.Errorf("resolving chat %q: not found, available in catalog: own-chat, private-other-team", p.ChatID) + } + passthrough := func(chatID string) (ChatResolveResult, error) { + return ChatResolveResult{ChatID: chatID}, nil + } + cfg := Config{Listen: ":0", BasePath: "/api/v1", Keys: []ResolvedKey{scoped, unscoped}, AsyncMode: true, DefaultRoutingMode: "catalog"} + srv := New(cfg, sendFn, passthrough) + + w := sendAs(srv, "narrow", "no-such-alias") + if strings.Contains(w.Body.String(), "private-other-team") { + t.Errorf("scoped key was handed the catalog: %s", w.Body.String()) + } + + w = sendAs(srv, "open", "no-such-alias") + if !strings.Contains(w.Body.String(), "private-other-team") { + t.Errorf("unscoped key lost the diagnostic detail: %s", w.Body.String()) + } +} + +// Distinguishable answers for "no such chat" and "not your chat" let a scoped +// key enumerate other teams' alias names by guessing and reading the status. +func TestKeyScope_UnknownAndForbiddenAreIndistinguishable(t *testing.T) { + scoped := ResolvedKey{Name: "b2c-at", Key: "narrow", Chats: []string{ownUUID}} + unscoped := ResolvedKey{Name: "any-app", Key: "open"} + srv := newScopeServer([]ResolvedKey{scoped, unscoped}) + + foreign := sendAs(srv, "narrow", "other-chat") + unknown := sendAs(srv, "narrow", "no-such-alias") + if foreign.Code != unknown.Code || foreign.Body.String() != unknown.Body.String() { + t.Errorf("scoped key can tell the two apart:\n foreign: %d %s\n unknown: %d %s", + foreign.Code, foreign.Body.String(), unknown.Code, unknown.Body.String()) + } + if foreign.Code != 403 { + t.Errorf("status = %d, want 403", foreign.Code) + } + + // An unscoped key keeps the diagnostic difference. + if a, b := sendAs(srv, "open", "no-such-alias"), sendAs(srv, "open", "other-chat"); a.Code == b.Code && a.Body.String() == b.Body.String() { + t.Error("unscoped key lost the distinction between unknown and allowed") + } +} + +// The same must hold on the async path, where an unresolvable alias surfaces as +// a delivery failure rather than a resolution error. +func TestKeyScope_AsyncUnknownAndForbiddenMatch(t *testing.T) { + scoped := ResolvedKey{Name: "b2c-at", Key: "narrow", Chats: []string{ownUUID}} + aliases := map[string]string{"own-chat": ownUUID, "other-chat": otherUUID} + sendFn := func(ctx context.Context, p *SendPayload) (string, error) { + id, ok := aliases[p.ChatID] + if !ok { + return "", fmt.Errorf("%w: not found, available in catalog: own-chat, private-other-team", ErrChatUnresolved) + } + if !ChatAllowed(ctx, id) { + return "", ErrChatNotAllowed + } + return "req-id", nil + } + passthrough := func(chatID string) (ChatResolveResult, error) { + return ChatResolveResult{ChatID: chatID}, nil + } + cfg := Config{Listen: ":0", BasePath: "/api/v1", Keys: []ResolvedKey{scoped}, AsyncMode: true, DefaultRoutingMode: "catalog"} + srv := New(cfg, sendFn, passthrough) + + foreign := sendAs(srv, "narrow", "other-chat") + unknown := sendAs(srv, "narrow", "no-such-alias") + if foreign.Code != unknown.Code || foreign.Body.String() != unknown.Body.String() { + t.Errorf("scoped key can tell the two apart:\n foreign: %d %s\n unknown: %d %s", + foreign.Code, foreign.Body.String(), unknown.Code, unknown.Body.String()) + } + if foreign.Code != 403 { + t.Errorf("status = %d, want 403", foreign.Code) + } +} + +func TestKeyScope_Alertmanager(t *testing.T) { + amCfg := testAlertmanagerConfig(t) + amCfg.DefaultChatID = ownUUID + scoped := ResolvedKey{Name: "b2c-at", Key: "narrow", Chats: []string{ownUUID}} + srv := newScopeServer([]ResolvedKey{scoped}, WithAlertmanager(amCfg)) + + body := alertmanagerPayload("firing", AlertItem{ + Status: "firing", + Labels: map[string]string{"alertname": "HighCPU", "severity": "critical"}, + Annotations: map[string]string{"summary": "CPU > 90%"}, + }) + + w := doRequest(srv, "POST", "/api/v1/alertmanager", strings.NewReader(body), + map[string]string{"Content-Type": "application/json", "Authorization": "Bearer narrow"}) + if w.Code != 200 { + t.Errorf("default chat in scope: status = %d, want 200 (body: %s)", w.Code, w.Body.String()) + } + + w = doRequest(srv, "POST", "/api/v1/alertmanager?chat_id="+otherUUID, strings.NewReader(body), + map[string]string{"Content-Type": "application/json", "Authorization": "Bearer narrow"}) + if w.Code != 403 { + t.Errorf("foreign chat via ?chat_id=: status = %d, want 403 (body: %s)", w.Code, w.Body.String()) + } +} + +func TestKeyScope_AliasListFiltered(t *testing.T) { + entries := []config.ChatEntry{ + {Name: "own-chat", ID: ownUUID}, + {Name: "other-chat", ID: otherUUID}, + } + scoped := ResolvedKey{Name: "b2c-at", Key: "narrow", Chats: []string{ownUUID}} + unscoped := ResolvedKey{Name: "any-app", Key: "open"} + srv := newScopeServer([]ResolvedKey{scoped, unscoped}, WithConfigInfo(nil, entries)) + + for _, tc := range []struct { + key string + want int + }{{"narrow", 1}, {"open", 2}} { + w := doRequest(srv, "GET", "/api/v1/chats/alias/list", nil, + map[string]string{"Authorization": "Bearer " + tc.key}) + var got []config.ChatEntry + if err := json.NewDecoder(w.Body).Decode(&got); err != nil { + t.Fatalf("decode: %v", err) + } + if len(got) != tc.want { + t.Errorf("key %s: %d entries, want %d (%v)", tc.key, len(got), tc.want, got) + } + } +} + +// --- regressions from review (2026-09-07) --- + +// Two keys sharing a name must not share permissions: the scope has to follow +// the credential that actually authenticated, not a name that may collide. +func TestKeyScope_DuplicateNamesDoNotShareScope(t *testing.T) { + first := ResolvedKey{Name: "team", Key: "first", Chats: []string{ownUUID}} + second := ResolvedKey{Name: "team", Key: "second", Chats: []string{otherUUID}} + srv := newScopeServer([]ResolvedKey{first, second}) + + if w := sendAs(srv, "first", otherUUID); w.Code != 403 { + t.Errorf("first key reached the second key's chat: status = %d, want 403", w.Code) + } + if w := sendAs(srv, "second", ownUUID); w.Code != 403 { + t.Errorf("second key reached the first key's chat: status = %d, want 403", w.Code) + } + if w := sendAs(srv, "first", ownUUID); w.Code != 200 { + t.Errorf("first key lost its own chat: status = %d, want 200", w.Code) + } + if w := sendAs(srv, "second", otherUUID); w.Code != 200 { + t.Errorf("second key lost its own chat: status = %d, want 200", w.Code) + } +} + +// The catalog that async delivery resolves against can disagree with the local +// config: the same alias may name a different chat there. Authorizing the local +// answer would authorize one chat while the message goes to another, so the +// send pipeline applies the scope to the address it is actually delivering to. +func TestKeyScope_AsyncCatalogDisagreesWithConfig(t *testing.T) { + const catalogUUID = "a55cdddb-a5b2-5901-9b8b-f4bfc522b448" + + // Local config: own-chat -> ownUUID. Catalog: own-chat -> catalogUUID. + entries := []config.ChatEntry{{Name: "own-chat", ID: ownUUID}} + scoped := ResolvedKey{Name: "b2c-at", Key: "narrow", Chats: []string{ownUUID}} + + var delivered string + sendFn := func(ctx context.Context, p *SendPayload) (string, error) { + chatID := p.ChatID + if !isUUID(chatID) { + chatID = catalogUUID + } + if !ChatAllowed(ctx, chatID) { + return "", ErrChatNotAllowed + } + delivered = chatID + return "req-id", nil + } + passthrough := func(chatID string) (ChatResolveResult, error) { + return ChatResolveResult{ChatID: chatID}, nil + } + cfg := Config{Listen: ":0", BasePath: "/api/v1", Keys: []ResolvedKey{scoped}, AsyncMode: true, DefaultRoutingMode: "catalog"} + srv := New(cfg, sendFn, passthrough, WithConfigInfo(nil, entries)) + + w := sendAs(srv, "narrow", "own-chat") + if w.Code != 403 { + t.Errorf("status = %d, want 403: the alias resolved to a chat outside the scope", w.Code) + } + if delivered != "" { + t.Errorf("message was delivered to %s despite the scope", delivered) + } + if strings.Contains(w.Body.String(), catalogUUID) { + t.Errorf("response named the catalog's chat: %s", w.Body.String()) + } +} + +// In async mode the chat resolver is a pass-through; the scope for aliases is +// applied by the send pipeline once the address is final. +func TestKeyScope_AsyncMode(t *testing.T) { + entries := []config.ChatEntry{ + {Name: "own-chat", ID: ownUUID}, + {Name: "other-chat", ID: otherUUID}, + } + scoped := ResolvedKey{Name: "b2c-at", Key: "narrow", Chats: []string{ownUUID}} + + cfg := Config{Listen: ":0", BasePath: "/api/v1", Keys: []ResolvedKey{scoped}, AsyncMode: true, DefaultRoutingMode: "mixed"} + // Stands in for the enqueue pipeline: resolves the alias, then applies the + // scope to the final address, as internal/cmd does before publishing. + aliases := map[string]string{"own-chat": ownUUID, "other-chat": otherUUID} + sendFn := func(ctx context.Context, p *SendPayload) (string, error) { + chatID := p.ChatID + if id, ok := aliases[chatID]; ok { + chatID = id + } + if !ChatAllowed(ctx, chatID) { + return "", ErrChatNotAllowed + } + return "req-id", nil + } + passthrough := func(chatID string) (ChatResolveResult, error) { + return ChatResolveResult{ChatID: chatID}, nil + } + srv := New(cfg, sendFn, passthrough, WithConfigInfo(nil, entries)) + + // A bare UUID in mixed routing mode needs an explicit bot; that rule is + // unrelated to scoping, so the UUID cases carry one. + sendAsync := func(chatID string) *httptest.ResponseRecorder { + body := strings.NewReader(fmt.Sprintf(`{"chat_id":%q,"message":"hi","bot":"b"}`, chatID)) + return doRequest(srv, "POST", "/api/v1/send", body, map[string]string{ + "Content-Type": "application/json", + "Authorization": "Bearer narrow", + }) + } + + tests := []struct { + name string + chatID string + want int + }{ + {"own alias resolves through the catalog", "own-chat", 202}, + {"own uuid", ownUUID, 202}, + {"foreign alias", "other-chat", 403}, + {"foreign uuid", otherUUID, 403}, + {"alias the pipeline cannot resolve", "catalog-only", 403}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if w := sendAsync(tc.chatID); w.Code != tc.want { + t.Errorf("status = %d, want %d (body: %s)", w.Code, tc.want, w.Body.String()) + } + }) + } +} + +// --- multi-chat fan-out (rebase onto main) --- + +// chat_id accepts a comma-separated list and delivery is best-effort per target, +// so a scope checked on one chat while the fan-out sends to several would be no +// scope at all. A list naming any chat outside the scope is refused whole. +func TestKeyScope_FanoutCannotSmuggleForeignChat(t *testing.T) { + scoped := ResolvedKey{Name: "b2c-at", Key: "narrow", Chats: []string{ownUUID}} + + var delivered []string + sendFn := func(ctx context.Context, p *SendPayload) (string, error) { + delivered = append(delivered, p.ChatID) + return "sync-id", nil + } + aliases := map[string]string{"own-chat": ownUUID, "other-chat": otherUUID} + resolver := func(chatID string) (ChatResolveResult, error) { + if id, ok := aliases[chatID]; ok { + return ChatResolveResult{ChatID: id}, nil + } + if chatID == ownUUID || chatID == otherUUID { + return ChatResolveResult{ChatID: chatID}, nil + } + return ChatResolveResult{}, fmt.Errorf("unknown chat alias %q, available: other-chat, own-chat", chatID) + } + cfg := Config{Listen: ":0", BasePath: "/api/v1", Keys: []ResolvedKey{scoped}} + srv := New(cfg, sendFn, resolver) + + for _, chatID := range []string{ + "own-chat,other-chat", + "other-chat,own-chat", + "own-chat," + otherUUID, + ownUUID + ",other-chat", + } { + t.Run(chatID, func(t *testing.T) { + delivered = nil + w := sendAs(srv, "narrow", chatID) + if w.Code != 403 { + t.Errorf("status = %d, want 403 (body: %s)", w.Code, w.Body.String()) + } + if len(delivered) != 0 { + t.Errorf("partial delivery to %v despite a foreign chat in the list", delivered) + } + }) + } + + t.Run("all targets in scope", func(t *testing.T) { + delivered = nil + if w := sendAs(srv, "narrow", "own-chat,"+ownUUID); w.Code != 200 { + t.Errorf("status = %d, want 200 (body: %s)", w.Code, w.Body.String()) + } + if len(delivered) == 0 { + t.Error("nothing delivered for an allowed list") + } + }) +} diff --git a/internal/server/server.go b/internal/server/server.go index 3b872e7..042c4bb 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -3,6 +3,7 @@ package server import ( "context" "crypto/tls" + "errors" "fmt" "log/slog" "net" @@ -21,10 +22,25 @@ import ( "github.com/lavr/express-botx/internal/mentions" ) +// ErrChatNotAllowed is returned by a send pipeline that resolved the final +// delivery address and found it outside the requesting key's chat scope. The +// send handler turns it into 403 rather than a delivery failure. +var ErrChatNotAllowed = errors.New("chat not allowed for this key") + +// ErrChatUnresolved is returned by a send pipeline that could not resolve the +// requested chat. A scoped caller is answered exactly as it would be for a chat +// outside its scope, keeping the two indistinguishable; an unscoped caller +// keeps the detailed failure. +var ErrChatUnresolved = errors.New("unknown chat") + // ResolvedKey is an API key with its secret resolved. type ResolvedKey struct { Name string Key string + // Chats is the key's chat scope as resolved UUIDs (lowercase). Empty means + // unrestricted. Aliases are resolved once at startup so request handling + // compares UUID against UUID and never re-reads the chat catalog. + Chats []string } // Config holds the server runtime configuration. @@ -58,8 +74,8 @@ type Server struct { cfg Config send SendFunc chats ChatResolver - keyMap map[string]string // key -> name - botNameSet map[string]bool // valid bot names for multi-bot mode + keyMap map[string]ResolvedKey // key value -> the key itself (name + chat scope) + botNameSet map[string]bool // valid bot names for multi-bot mode apm apm.Provider errTracker errtrack.Tracker botEntries []config.BotEntry // for GET /bot/list @@ -246,7 +262,7 @@ func New(cfg Config, sendFn SendFunc, chatResolver ChatResolver, opts ...Option) cfg: cfg, send: sendFn, chats: chatResolver, - keyMap: make(map[string]string, len(cfg.Keys)), + keyMap: make(map[string]ResolvedKey, len(cfg.Keys)), botNameSet: make(map[string]bool, len(cfg.BotNames)), callbackCtx: cbCtx, callbackCancel: cbCancel, @@ -256,7 +272,12 @@ func New(cfg Config, sendFn SendFunc, chatResolver ChatResolver, opts ...Option) s.tlsReloader = newCertReloader(cfg.TLS.CertFile, cfg.TLS.KeyFile, cfg.TLS.ReloadInterval) } for _, k := range cfg.Keys { - s.keyMap[k.Key] = k.Name + scope := make([]string, len(k.Chats)) + for i, c := range k.Chats { + scope[i] = strings.ToLower(c) + } + k.Chats = scope + s.keyMap[k.Key] = k } for _, name := range cfg.BotNames { s.botNameSet[name] = true