mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-21 16:18:55 +00:00
fix(gateway): bridge nested DingTalk allowed_users into auth env
The DingTalk docs offer gateway.platforms.dingtalk.extra.allowed_users
as the config.yaml alternative to DINGTALK_ALLOWED_USERS. The adapter
honors it (_load_allowed_users reads PlatformConfig.extra), but gateway
authorization (_is_user_authorized in gateway/authz_mixin.py) only
consults the env var, and load_gateway_config() bridged the allowlist
to the env var only from a top-level dingtalk: block. A nested-only
allowlist therefore passed the adapter and was then denied at the
gateway - listed users fell through to pairing/default-deny in DMs.
Extend the DingTalk YAML->env bridge to fall back to the merged nested
platform config (gateway.platforms / platforms), mirroring the existing
platforms.discord.extra.allow_from precedent. Precedence is unchanged:
an explicit DINGTALK_ALLOWED_USERS env var still wins, then the
top-level dingtalk: block, then the nested extra.
Also correct the docs' claim that the two allowlists are "merged" when
both are set - that behavior never existed (the doc line came from a
docs-only sweep); the effective result is the intersection of the two
gates, so the docs now recommend configuring one or the other.
Repro (before): config.yaml containing only the nested allowlist ->
adapter._is_user_allowed("user-id-1") is True but
runner._is_user_authorized(...) is False. After: both True; unlisted
users are still denied.
This commit is contained in:
parent
330b224525
commit
222772ad61
4 changed files with 185 additions and 2 deletions
|
|
@ -1660,6 +1660,31 @@ def _apply_yaml_config(yaml_cfg: dict, dingtalk_cfg: dict) -> dict | None:
|
|||
ac = ",".join(str(v) for v in ac)
|
||||
os.environ["DINGTALK_ALLOWED_CHATS"] = str(ac)
|
||||
allowed = dingtalk_cfg.get("allowed_users")
|
||||
if allowed is None:
|
||||
# Fall back to the documented nested paths (#44928). The docs
|
||||
# (website/docs/user-guide/messaging/dingtalk.md) configure the
|
||||
# allowlist at gateway.platforms.dingtalk.extra.allowed_users; the
|
||||
# adapter reads it from PlatformConfig.extra, but gateway
|
||||
# authorization (_is_user_authorized in gateway/authz_mixin.py)
|
||||
# only consults DINGTALK_ALLOWED_USERS — without this bridge a
|
||||
# nested-only allowlist passes the adapter and is then denied at
|
||||
# the gateway. Check this block's own extra first (the dispatch
|
||||
# loop passes the platforms block here when no top-level
|
||||
# ``dingtalk:`` section exists), then both nested containers.
|
||||
_extra = dingtalk_cfg.get("extra")
|
||||
if isinstance(_extra, dict):
|
||||
allowed = _extra.get("allowed_users")
|
||||
if allowed is None:
|
||||
_gw = yaml_cfg.get("gateway")
|
||||
_gw_platforms = _gw.get("platforms") if isinstance(_gw, dict) else None
|
||||
for _container in (_gw_platforms, yaml_cfg.get("platforms")):
|
||||
if not isinstance(_container, dict):
|
||||
continue
|
||||
_dt = _container.get("dingtalk")
|
||||
_dt_extra = _dt.get("extra") if isinstance(_dt, dict) else None
|
||||
if isinstance(_dt_extra, dict) and _dt_extra.get("allowed_users") is not None:
|
||||
allowed = _dt_extra.get("allowed_users")
|
||||
break
|
||||
if allowed is not None and not os.getenv("DINGTALK_ALLOWED_USERS"):
|
||||
if isinstance(allowed, list):
|
||||
allowed = ",".join(str(v) for v in allowed)
|
||||
|
|
|
|||
|
|
@ -1134,6 +1134,164 @@ class TestLoadGatewayConfig:
|
|||
]
|
||||
assert os.environ.get("DISCORD_ALLOWED_USERS") == "123456789012345678"
|
||||
|
||||
def test_bridges_nested_gateway_platforms_dingtalk_allowed_users_to_env(self, tmp_path, monkeypatch):
|
||||
"""gateway.platforms.dingtalk.extra.allowed_users must reach
|
||||
DINGTALK_ALLOWED_USERS — it's the documented config.yaml alternative
|
||||
to the env var (website/docs/user-guide/messaging/dingtalk.md), the
|
||||
adapter reads it from PlatformConfig.extra, but gateway auth
|
||||
(_is_user_authorized) only consults the env var.
|
||||
"""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
config_path = hermes_home / "config.yaml"
|
||||
config_path.write_text(
|
||||
"gateway:\n"
|
||||
" platforms:\n"
|
||||
" dingtalk:\n"
|
||||
" enabled: true\n"
|
||||
" extra:\n"
|
||||
" allowed_users:\n"
|
||||
" - user-id-1\n"
|
||||
" - user-id-2\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
monkeypatch.delenv("DINGTALK_ALLOWED_USERS", raising=False)
|
||||
|
||||
config = load_gateway_config()
|
||||
|
||||
assert config.platforms[Platform.DINGTALK].extra["allowed_users"] == [
|
||||
"user-id-1",
|
||||
"user-id-2",
|
||||
]
|
||||
assert os.environ.get("DINGTALK_ALLOWED_USERS") == "user-id-1,user-id-2"
|
||||
|
||||
def test_bridges_platforms_dingtalk_extra_allowed_users_to_env(self, tmp_path, monkeypatch):
|
||||
"""platforms.dingtalk.extra.allowed_users should reach DINGTALK_ALLOWED_USERS too."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
config_path = hermes_home / "config.yaml"
|
||||
config_path.write_text(
|
||||
"platforms:\n"
|
||||
" dingtalk:\n"
|
||||
" extra:\n"
|
||||
" allowed_users:\n"
|
||||
" - manager1234\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
monkeypatch.delenv("DINGTALK_ALLOWED_USERS", raising=False)
|
||||
|
||||
config = load_gateway_config()
|
||||
|
||||
assert config.platforms[Platform.DINGTALK].extra["allowed_users"] == [
|
||||
"manager1234",
|
||||
]
|
||||
assert os.environ.get("DINGTALK_ALLOWED_USERS") == "manager1234"
|
||||
|
||||
def test_dingtalk_allowed_users_env_takes_precedence_over_config_yaml(self, tmp_path, monkeypatch):
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
config_path = hermes_home / "config.yaml"
|
||||
config_path.write_text(
|
||||
"gateway:\n"
|
||||
" platforms:\n"
|
||||
" dingtalk:\n"
|
||||
" extra:\n"
|
||||
" allowed_users:\n"
|
||||
" - config-user\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
monkeypatch.setenv("DINGTALK_ALLOWED_USERS", "env-user")
|
||||
|
||||
load_gateway_config()
|
||||
|
||||
assert os.environ.get("DINGTALK_ALLOWED_USERS") == "env-user"
|
||||
|
||||
def test_top_level_dingtalk_allowed_users_wins_over_nested_extra(self, tmp_path, monkeypatch):
|
||||
"""The legacy top-level dingtalk: block keeps precedence over the
|
||||
nested platform extra when both define an allowlist."""
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
config_path = hermes_home / "config.yaml"
|
||||
config_path.write_text(
|
||||
"dingtalk:\n"
|
||||
" allowed_users:\n"
|
||||
" - top-level-user\n"
|
||||
"gateway:\n"
|
||||
" platforms:\n"
|
||||
" dingtalk:\n"
|
||||
" extra:\n"
|
||||
" allowed_users:\n"
|
||||
" - nested-user\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
monkeypatch.delenv("DINGTALK_ALLOWED_USERS", raising=False)
|
||||
|
||||
load_gateway_config()
|
||||
|
||||
assert os.environ.get("DINGTALK_ALLOWED_USERS") == "top-level-user"
|
||||
|
||||
def test_nested_dingtalk_allowlist_authorizes_listed_user_only(self, tmp_path, monkeypatch):
|
||||
"""E2E for the documented setup: a nested-only allowlist must
|
||||
authorize the listed user at the gateway and still deny others.
|
||||
|
||||
Before the bridge existed, the listed user passed the adapter's
|
||||
_is_user_allowed() but _is_user_authorized() fell through to
|
||||
default-deny because DINGTALK_ALLOWED_USERS was never populated.
|
||||
"""
|
||||
from types import SimpleNamespace
|
||||
|
||||
from gateway.run import GatewayRunner
|
||||
from gateway.session import SessionSource
|
||||
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
config_path = hermes_home / "config.yaml"
|
||||
config_path.write_text(
|
||||
"gateway:\n"
|
||||
" platforms:\n"
|
||||
" dingtalk:\n"
|
||||
" enabled: true\n"
|
||||
" extra:\n"
|
||||
" allowed_users:\n"
|
||||
" - user-id-1\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(hermes_home))
|
||||
for var in (
|
||||
"DINGTALK_ALLOWED_USERS",
|
||||
"DINGTALK_ALLOW_ALL_USERS",
|
||||
"GATEWAY_ALLOWED_USERS",
|
||||
"GATEWAY_ALLOW_ALL_USERS",
|
||||
):
|
||||
monkeypatch.delenv(var, raising=False)
|
||||
|
||||
config = load_gateway_config()
|
||||
|
||||
runner = object.__new__(GatewayRunner)
|
||||
runner.pairing_store = SimpleNamespace(is_approved=lambda *_a, **_kw: False)
|
||||
runner.config = config
|
||||
|
||||
def _dm_source(user_id):
|
||||
return SessionSource(
|
||||
platform=Platform.DINGTALK,
|
||||
chat_id="dm-1",
|
||||
chat_type="dm",
|
||||
user_id=user_id,
|
||||
user_name="someone",
|
||||
)
|
||||
|
||||
assert runner._is_user_authorized(_dm_source("user-id-1")) is True
|
||||
assert runner._is_user_authorized(_dm_source("intruder")) is False
|
||||
|
||||
def test_bridges_quoted_false_platform_enabled_from_config_yaml(self, tmp_path, monkeypatch):
|
||||
hermes_home = tmp_path / ".hermes"
|
||||
hermes_home.mkdir()
|
||||
|
|
|
|||
|
|
@ -154,7 +154,7 @@ gateway:
|
|||
|
||||
- `group_sessions_per_user: true` keeps each participant's context isolated inside shared group chats
|
||||
- `require_mention: true` prevents the bot from responding to every group message — it only answers when someone @-mentions it
|
||||
- `allowed_users` under `dingtalk.extra` is an alternative to `DINGTALK_ALLOWED_USERS`; if both are set, they're merged
|
||||
- `allowed_users` under `dingtalk.extra` is an alternative to `DINGTALK_ALLOWED_USERS`; set one or the other (if both are set, only users present in both lists are authorized)
|
||||
|
||||
### Start the Gateway
|
||||
|
||||
|
|
|
|||
|
|
@ -154,7 +154,7 @@ gateway:
|
|||
|
||||
- `group_sessions_per_user: true` 在共享群聊中保持每个参与者的上下文隔离
|
||||
- `require_mention: true` 防止机器人响应每条群消息——仅在有人 @提及 时才回答
|
||||
- `dingtalk.extra` 下的 `allowed_users` 是 `DINGTALK_ALLOWED_USERS` 的替代方式;若两者同时设置,则合并生效
|
||||
- `dingtalk.extra` 下的 `allowed_users` 是 `DINGTALK_ALLOWED_USERS` 的替代方式;两者择一配置(若同时设置,只有同时出现在两个列表中的用户才会被授权)
|
||||
|
||||
### 启动 Gateway
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue