Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions docs/integrations.md
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,70 @@ POST /api/v1/gitlab?chat_id= X-Gitlab-Token: <team-a-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-токена.
Expand Down
83 changes: 78 additions & 5 deletions internal/cmd/serve.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"os/signal"
"path/filepath"
"strconv"
"strings"
"syscall"
"time"

Expand Down Expand Up @@ -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)
}
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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{
Expand Down
95 changes: 95 additions & 0 deletions internal/cmd/serve_scope_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
28 changes: 21 additions & 7 deletions internal/cmd/server_apikey.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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)
}
Expand All @@ -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()
}

Expand Down Expand Up @@ -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 {
Expand Down
Loading