Skip to content

Commit 0f940ca

Browse files
chaodu-agentchaodu-agent
andauthored
feat(telegram): add allow_all_users/allowed_users to [telegram] config (#1297)
* feat(telegram): add allow_all_users/allowed_users to [telegram] config Discord and Slack have had allowed_users in their first-class config sections for a while, but [telegram] never did - the only way to restrict who can message a Telegram bot was the shared GATEWAY_* env vars (GATEWAY_ALLOW_ALL_USERS/GATEWAY_ALLOWED_USERS), which default to deny-all (per the identity-trust-none ADR) and aren't Telegram-specific. Add allow_all_users/allowed_users to TelegramConfig, matching the same resolution convention already used for every other [telegram] field and for Discord's allowed_users: config value wins, falls back to a TELEGRAM_* env var (TELEGRAM_ALLOW_ALL_USERS / TELEGRAM_ALLOWED_USERS, comma-separated), then auto-detects allow_all_users from whether the resolved list is empty (same convention as [discord]/[slack]). Wire the resolved values into main.rs's PlatformTrustConfigs registry as a Telegram-specific override, mirroring exactly how Discord's own block already overrides the shared GATEWAY_* L3 defaults - added right after the existing Discord override. Now a bot operator can write: [telegram] bot_token = "${TELEGRAM_BOT_TOKEN}" allowed_users = ["176096071"] instead of routing a plain user ID through spec.secrets/Secrets Manager (oabctl's manifest has no plain env var field) or opening the bot to allow_all_users=true just to admit one person. Testing: cargo build/clippy -D warnings/test clean on macmini for the whole workspace (openab-core: 48 tests incl. 5 new allowed_users scenarios in telegram_resolve_all_scenarios; openab binary: 15 tests). No regressions. Fixed one now-incomplete TelegramConfig struct literal in an existing test (missing the two new fields). * docs(telegram): document allowed_users/allow_all_users config Add the new [telegram] user access control fields to: - Unified mode env vars table - Full example config block - Field precedence table - Gateway Environment Variables table - New 'User Access Control' section with auto-detect logic explanation * feat(telegram): default allow_all_users to false (deny-all) Align [telegram] user gating with the identity-trust-none ADR: - Empty allowed_users + no explicit flag → deny all (was: allow all) - Users must now explicitly set allow_all_users = true or list specific IDs in allowed_users to admit senders - Matches the shared GATEWAY_ALLOW_ALL_USERS default (false) This is not a breaking change for existing deployments because: - [telegram] config section is brand new (introduced in this PR) - Existing env-only deployments use GATEWAY_ALLOW_ALL_USERS which already defaults to deny-all Added Scenario 12 test: explicit allow_all_users = true opt-in. * fix(review): env-only bypass + Option<Vec> for config-authoritative override - F1: Resolve Telegram trust override even when [telegram] section is absent but TELEGRAM_ALLOWED_USERS / TELEGRAM_ALLOW_ALL_USERS env vars are set. Previously, env-only deployments (no config.toml section) silently ignored these env vars and fell back to GATEWAY_* defaults. - F2: Change TelegramConfig.allowed_users from Vec<String> (with #[serde(default)]) to Option<Vec<String>>. This lets serde distinguish omitted (None → fall back to env) from explicitly empty (Some([]) → deny all, config-authoritative). Fixes the case where allowed_users=[] in config.toml could not override a non-empty TELEGRAM_ALLOWED_USERS env var. - Add Scenario 13 test: explicit empty list overrides env var. Addresses review findings from 法師團隊 on PR #1297. * fix(review): empty-string env fail-open, doc clarity, gateway scope - TELEGRAM_ALLOW_ALL_USERS="" now resolves to false (deny-all) instead of true. Adds .filter(|v| !v.is_empty()) before parsing to prevent accidental fail-open from empty env var. - Added doc comment clarifying that allow_all_users=true bypasses the allowed_users list entirely. - Marked TELEGRAM_ALLOWED_USERS/TELEGRAM_ALLOW_ALL_USERS as "Unified binary only" in the Gateway env vars table — standalone gateway uses [gateway].allowed_users instead. - Added Scenario 14 test: empty-string env var must resolve to false. --------- Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com> Co-authored-by: chaodu-agent <chaodu-agent@openab.dev>
1 parent e804673 commit 0f940ca

3 files changed

Lines changed: 169 additions & 0 deletions

File tree

crates/openab-core/src/config.rs

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,19 @@ pub struct TelegramConfig {
605605
/// Webhook mount path. Env fallback: `TELEGRAM_WEBHOOK_PATH`
606606
/// (default `/webhook/telegram`).
607607
pub webhook_path: Option<String>,
608+
/// Explicit flag: true = allow all users, false = check `allowed_users`.
609+
/// When not set, defaults to `false` (deny-all, per identity-trust-none ADR).
610+
/// Set `true` explicitly to allow all users. Env fallback:
611+
/// `TELEGRAM_ALLOW_ALL_USERS` (empty string treated as unset).
612+
///
613+
/// **Note:** When this resolves to `true`, the `allowed_users` list is
614+
/// bypassed entirely — all users are permitted regardless of list contents.
615+
pub allow_all_users: Option<bool>,
616+
/// Telegram user IDs allowed to interact with the bot. Only checked when
617+
/// `allow_all_users` resolves to `false`. Env fallback:
618+
/// `TELEGRAM_ALLOWED_USERS` (comma-separated).
619+
/// `None` = not set (fall back to env); `Some([])` = explicit empty (deny all).
620+
pub allowed_users: Option<Vec<String>>,
608621
}
609622

610623
/// Fully resolved Telegram settings (config → env → default applied).
@@ -618,6 +631,8 @@ pub struct ResolvedTelegram {
618631
pub rich_messages: bool,
619632
pub streaming: Option<bool>,
620633
pub webhook_path: String,
634+
pub allow_all_users: bool,
635+
pub allowed_users: Vec<String>,
621636
}
622637

623638
impl TelegramConfig {
@@ -627,6 +642,15 @@ impl TelegramConfig {
627642
/// unset env vars, so `bot_token = "${UNSET_VAR}"` correctly falls through
628643
/// to the `TELEGRAM_BOT_TOKEN` env fallback rather than holding `Some("")`.
629644
pub fn resolve(&self) -> ResolvedTelegram {
645+
let allowed_users: Vec<String> = match &self.allowed_users {
646+
Some(list) => list.clone(),
647+
None => std::env::var("TELEGRAM_ALLOWED_USERS")
648+
.unwrap_or_default()
649+
.split(',')
650+
.map(|s| s.trim().to_string())
651+
.filter(|s| !s.is_empty())
652+
.collect(),
653+
};
630654
ResolvedTelegram {
631655
bot_token: self
632656
.bot_token
@@ -658,6 +682,14 @@ impl TelegramConfig {
658682
.cloned()
659683
.or_else(|| std::env::var("TELEGRAM_WEBHOOK_PATH").ok())
660684
.unwrap_or_else(|| "/webhook/telegram".into()),
685+
allow_all_users: self.allow_all_users.unwrap_or_else(|| {
686+
std::env::var("TELEGRAM_ALLOW_ALL_USERS")
687+
.ok()
688+
.filter(|v| !v.is_empty())
689+
.map(|v| v != "0" && !v.eq_ignore_ascii_case("false"))
690+
.unwrap_or(false)
691+
}),
692+
allowed_users,
661693
}
662694
}
663695
}
@@ -1521,6 +1553,8 @@ mod tests {
15211553
"TELEGRAM_RICH_MESSAGES",
15221554
"TELEGRAM_STREAMING",
15231555
"TELEGRAM_WEBHOOK_PATH",
1556+
"TELEGRAM_ALLOW_ALL_USERS",
1557+
"TELEGRAM_ALLOWED_USERS",
15241558
] {
15251559
std::env::remove_var(k);
15261560
}
@@ -1534,6 +1568,8 @@ mod tests {
15341568
rich_messages: Some(false),
15351569
streaming: Some(true),
15361570
webhook_path: Some("/custom/tg".into()),
1571+
allow_all_users: None,
1572+
allowed_users: None,
15371573
};
15381574
let r = cfg.resolve();
15391575
assert_eq!(r.bot_token.as_deref(), Some("cfg-token"));
@@ -1608,6 +1644,77 @@ mod tests {
16081644
assert_eq!(r.secret_token.as_deref(), Some("real-secret"));
16091645
assert_eq!(r.webhook_path, "/webhook/telegram"); // env not set → default
16101646

1647+
// --- Scenario 7: allowed_users config wins over env; the separate
1648+
// allow_all_users flag resolves independently (config → env →
1649+
// auto-detect) and here falls through to the env var since the
1650+
// config struct didn't set it explicitly ---
1651+
std::env::set_var("TELEGRAM_ALLOW_ALL_USERS", "true");
1652+
std::env::set_var("TELEGRAM_ALLOWED_USERS", "999"); // must be ignored — config list wins
1653+
let cfg = TelegramConfig {
1654+
allowed_users: Some(vec!["111".into(), "222".into()]),
1655+
..Default::default()
1656+
};
1657+
let r = cfg.resolve();
1658+
assert_eq!(r.allowed_users, vec!["111".to_string(), "222".to_string()]);
1659+
assert!(r.allow_all_users); // from TELEGRAM_ALLOW_ALL_USERS=true, not auto-detect
1660+
std::env::remove_var("TELEGRAM_ALLOW_ALL_USERS");
1661+
std::env::remove_var("TELEGRAM_ALLOWED_USERS");
1662+
1663+
// --- Scenario 8: empty list + no explicit flag → allow_all_users
1664+
// defaults to false (identity-trust-none: deny-all by default) ---
1665+
let r = TelegramConfig::default().resolve();
1666+
assert!(r.allowed_users.is_empty());
1667+
assert!(!r.allow_all_users);
1668+
1669+
// --- Scenario 9: non-empty list + no explicit flag → auto-detects
1670+
// false (deny-all-except-list) ---
1671+
let cfg = TelegramConfig {
1672+
allowed_users: Some(vec!["176096071".into()]),
1673+
..Default::default()
1674+
};
1675+
let r = cfg.resolve();
1676+
assert_eq!(r.allowed_users, vec!["176096071".to_string()]);
1677+
assert!(!r.allow_all_users);
1678+
1679+
// --- Scenario 10: TELEGRAM_ALLOWED_USERS env fallback (comma-separated,
1680+
// trimmed) when config list is empty ---
1681+
std::env::set_var("TELEGRAM_ALLOWED_USERS", " 111 , 222,333 ");
1682+
let r = TelegramConfig::default().resolve();
1683+
assert_eq!(r.allowed_users, vec!["111".to_string(), "222".to_string(), "333".to_string()]);
1684+
assert!(!r.allow_all_users); // default false (deny-all)
1685+
std::env::remove_var("TELEGRAM_ALLOWED_USERS");
1686+
1687+
// --- Scenario 11: explicit allow_all_users = false matches
1688+
// the deny-all default (no-op but valid config) ---
1689+
let cfg = TelegramConfig { allow_all_users: Some(false), ..Default::default() };
1690+
assert!(!cfg.resolve().allow_all_users);
1691+
1692+
// --- Scenario 12: explicit allow_all_users = true opts in to
1693+
// allow-all (overrides deny-all default) ---
1694+
let cfg = TelegramConfig { allow_all_users: Some(true), ..Default::default() };
1695+
assert!(cfg.resolve().allow_all_users);
1696+
1697+
// --- Scenario 13: explicit empty list (Some([])) overrides
1698+
// TELEGRAM_ALLOWED_USERS env — config-authoritative even when
1699+
// the list is empty (deny all, regardless of env) ---
1700+
std::env::set_var("TELEGRAM_ALLOWED_USERS", "999,888");
1701+
let cfg = TelegramConfig {
1702+
allowed_users: Some(vec![]),
1703+
..Default::default()
1704+
};
1705+
let r = cfg.resolve();
1706+
assert!(r.allowed_users.is_empty()); // explicit empty wins over env
1707+
assert!(!r.allow_all_users);
1708+
std::env::remove_var("TELEGRAM_ALLOWED_USERS");
1709+
1710+
// --- Scenario 14: TELEGRAM_ALLOW_ALL_USERS="" (empty string) must
1711+
// resolve to false (deny-all), not true. Empty string is treated
1712+
// as unset to avoid accidental fail-open. ---
1713+
std::env::set_var("TELEGRAM_ALLOW_ALL_USERS", "");
1714+
let r = TelegramConfig::default().resolve();
1715+
assert!(!r.allow_all_users); // empty string = unset = deny-all
1716+
std::env::remove_var("TELEGRAM_ALLOW_ALL_USERS");
1717+
16111718
// --- Cleanup ---
16121719
for k in [
16131720
"TELEGRAM_BOT_TOKEN",
@@ -1616,6 +1723,8 @@ mod tests {
16161723
"TELEGRAM_RICH_MESSAGES",
16171724
"TELEGRAM_STREAMING",
16181725
"TELEGRAM_WEBHOOK_PATH",
1726+
"TELEGRAM_ALLOW_ALL_USERS",
1727+
"TELEGRAM_ALLOWED_USERS",
16191728
] {
16201729
std::env::remove_var(k);
16211730
}

docs/telegram.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ Set environment variables:
3434
| `TELEGRAM_BOT_USERNAME` | No | Bot username for @mention gating |
3535
| `TELEGRAM_RICH_MESSAGES` | No | `true` (default) for rich formatting |
3636
| `TELEGRAM_STREAMING` | No | follows `TELEGRAM_RICH_MESSAGES` | Stream replies via rich message drafts. Defaults to `true` when rich messages are enabled, `false` otherwise. Set explicitly to override |
37+
| `TELEGRAM_ALLOWED_USERS` | No | Comma-separated Telegram user IDs allowed to interact (empty = deny all) |
38+
| `TELEGRAM_ALLOW_ALL_USERS` | No | `true`/`false` — defaults to `false` (deny-all). Set `true` to allow everyone. |
3739
| `GATEWAY_LISTEN` | No | Listen address (default: `0.0.0.0:8080`) |
3840

3941
OAB config (`config.toml`):
@@ -87,6 +89,8 @@ trusted_source_only = true # reject requests outside Telegram's IP subnets
8789
rich_messages = true # sendRichMessage rendering (default true)
8890
streaming = true # override; defaults to follow rich_messages
8991
webhook_path = "/webhook/telegram"
92+
allowed_users = ["12345678"] # restrict to specific Telegram user IDs
93+
# allow_all_users = true # set true to allow everyone (default: false)
9094
```
9195

9296
**Precedence (per field):** `[telegram]` value (with `${}` expansion) → `TELEGRAM_*` env var → built-in default. This is config-authoritative and matches `[discord]`/`[slack]`. Any field you omit falls back to its env var, so existing env-only deployments keep working unchanged.
@@ -99,6 +103,8 @@ webhook_path = "/webhook/telegram"
99103
| `rich_messages` | `TELEGRAM_RICH_MESSAGES` | `true` |
100104
| `streaming` | `TELEGRAM_STREAMING` | follows `rich_messages` |
101105
| `webhook_path` | `TELEGRAM_WEBHOOK_PATH` | `/webhook/telegram` |
106+
| `allowed_users` | `TELEGRAM_ALLOWED_USERS` (comma-separated) | `[]` (deny all if empty) |
107+
| `allow_all_users` | `TELEGRAM_ALLOW_ALL_USERS` | `false` (deny-all) |
102108

103109
> **Tip**: You can run a pure config-only deployment — no `TELEGRAM_*` env vars needed. Just set `bot_token = "your-token"` directly in `[telegram]` and the adapter will activate from config alone.
104110
@@ -117,6 +123,25 @@ webhook_path = "/webhook/telegram"
117123
> See [secrets-management.md](secrets-management.md) for full documentation.
118124
119125
126+
### User Access Control
127+
128+
Restrict which Telegram users can interact with the bot using `allowed_users`:
129+
130+
```toml
131+
[telegram]
132+
bot_token = "${TELEGRAM_BOT_TOKEN}"
133+
allowed_users = ["12345678", "87654321"] # only these user IDs can chat
134+
```
135+
136+
**Default behavior** (identity-trust-none):
137+
- No config → `allow_all_users` defaults to `false` → bot denies all users
138+
- Set `allowed_users = ["12345678"]` → only listed IDs can chat
139+
- Set `allow_all_users = true` → open to everyone (opt-in)
140+
141+
**Resolution order:** config value → `TELEGRAM_ALLOWED_USERS` env var (comma-separated) → empty (deny all).
142+
143+
> **Finding your Telegram user ID:** Send `/start` to [@userinfobot](https://t.me/userinfobot) or use the `getUpdates` API after messaging your bot.
144+
120145
### Set the Webhook
121146

122147
```bash
@@ -350,6 +375,8 @@ Set `TELEGRAM_RICH_MESSAGES=false` to disable rich messages and use legacy `send
350375
| `TELEGRAM_SECRET_TOKEN` | No | — | Webhook signature validation |
351376
| `TELEGRAM_RICH_MESSAGES` | No | `true` | Use `sendRichMessage` for tables/headings/long content (Bot API 10.1+). Set `false` to opt out. |
352377
| `TELEGRAM_STREAMING` | No | follows `TELEGRAM_RICH_MESSAGES` | Stream replies live via `sendRichMessageDraft`. Defaults to `true` when rich messages are enabled, `false` otherwise. Set `false` for send-once mode (single final message). |
378+
| `TELEGRAM_ALLOWED_USERS` | No | — | Comma-separated Telegram user IDs allowed to interact with the bot. Empty = deny all. **Unified binary only** — standalone gateway uses `[gateway].allowed_users` instead. |
379+
| `TELEGRAM_ALLOW_ALL_USERS` | No | `false` | Explicit flag: `true` = allow all users, `false` = check `allowed_users`. Defaults to `false` (deny-all, per identity-trust-none ADR). **Unified binary only.** |
353380
| `GATEWAY_WS_TOKEN` | No | — | WebSocket auth token |
354381
| `GATEWAY_LISTEN` | No | `0.0.0.0:8080` | Listen address |
355382
| `TELEGRAM_WEBHOOK_PATH` | No | `/webhook/telegram` | Webhook endpoint path |

src/main.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,39 @@ async fn main() -> anyhow::Result<()> {
316316
),
317317
);
318318
}
319+
320+
// Telegram: L3 (identity) mirrors the resolved
321+
// [telegram].allow_all_users/allowed_users, so config.toml can
322+
// restrict who can message the bot without needing
323+
// GATEWAY_ALLOW_ALL_USERS/GATEWAY_ALLOWED_USERS env vars. L2
324+
// (channels) has no Telegram-specific concept distinct from the
325+
// generic gateway model, so it stays on the shared GATEWAY_* values
326+
// set above.
327+
//
328+
// Also resolves when running env-only (no [telegram] section but
329+
// TELEGRAM_BOT_TOKEN set), so TELEGRAM_ALLOWED_USERS /
330+
// TELEGRAM_ALLOW_ALL_USERS are honored in pure-env deployments.
331+
let telegram_resolved = if let Some(t) = &cfg.telegram {
332+
Some(t.resolve())
333+
} else if std::env::var("TELEGRAM_ALLOWED_USERS").is_ok()
334+
|| std::env::var("TELEGRAM_ALLOW_ALL_USERS").is_ok()
335+
{
336+
Some(config::TelegramConfig::default().resolve())
337+
} else {
338+
None
339+
};
340+
if let Some(r) = telegram_resolved {
341+
reg.insert(
342+
"telegram",
343+
TrustConfig::new(
344+
Some(allow_all_channels),
345+
allowed_channels.clone(),
346+
None,
347+
Some(r.allow_all_users),
348+
r.allowed_users,
349+
),
350+
);
351+
}
319352
reg
320353
};
321354

0 commit comments

Comments
 (0)