ouyangbo/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 9 Packages Projects Releases Wiki Activity

fix(gateway): stop multiplex allowlist leak and bot-relay python -c injection

Browse Source 
_auth_env fell through to os.environ on a scoped miss, so one profile
could inherit another profile's allowlists and allow-all flags.

bot_relay.waiter_command put connection_id into python -c source. A
quote in the id broke the waiter. A crafted id could run extra Python
in the sender gateway.
This commit is contained in:
Adolanium
2026-08-23 22:19:23 +03:00
committed by Teknium
parent 57649294be
commit 2912c36aa4
5 changed files with 139 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
gateway/authz_mixin.py
+16 -19
View File
@@ -28,28 +28,12 @@ from gateway.whatsapp_identity import (
)
def _auth_env(name: str, default: str = "") -> str:
"""Read allowlist/auth env; prefer profile secret_scope under multiplex."""
if not name:
return default
try:
from agent.secret_scope import get_secret
val = get_secret(name)
if val is not None and str(val).strip():
return str(val).strip()
except Exception:
pass
return (os.getenv(name) or default).strip()
def _platform_gate_env(name: str, default: str = "") -> str:
"""Read a platform allow/deny gate env var with per-profile isolation.
Like ``_auth_env`` but authoritative under multiplex: when a profile
secret scope is installed AND multiplexing is active, a key absent from
the scope returns ``default`` instead of falling through to
``os.environ``. Under multiplex the process env may hold ANOTHER
When a profile secret scope is installed AND multiplexing is active, a
key absent from the scope returns ``default`` instead of falling through
to ``os.environ``. Under multiplex the process env may hold ANOTHER
profile's first-writer-bridged value (the YAML→env bridges in the
Discord/Telegram adapters' ``_apply_yaml_config`` are first-writer-wins),
so falling through would leak profile A's allowlist into profile B
@@ -72,6 +56,19 @@ def _platform_gate_env(name: str, default: str = "") -> str:
return (os.getenv(name) or default).strip()
def _auth_env(name: str, default: str = "") -> str:
"""Read allowlist/auth env with per-profile isolation under multiplex.
Same rules as ``_platform_gate_env``: a scoped miss under multiplex
returns ``default`` and does not fall through to ``os.environ``. The
process env may hold another profile's first-writer-bridged value, so
a fallthrough would leak allowlists and allow-all flags across profiles
(issue #72348). Single-profile deployments keep the legacy
``os.getenv`` read.
"""
return _platform_gate_env(name, default)
def _coerce_allow_set(raw) -> set[str]:
"""Parse allowlist values from config or env var into a set of strings.
tests/agent/test_secret_scope_tier1_migration.py
+31
View File
@@ -94,6 +94,37 @@ class TestAuthzPlatformGateEnv:
assert _platform_gate_env("GATEWAY_ALLOWED_USERS") == "42"
class TestAuthzAuthEnv:
"""_auth_env must follow _platform_gate_env isolation (no os.environ
fallthrough on a scoped miss under multiplex)."""
def test_scoped_value_wins(self, monkeypatch):
from gateway.authz_mixin import _auth_env
monkeypatch.setenv("TELEGRAM_ALLOWED_USERS", "111")
ss.set_multiplex_active(True)
with _Scope({"TELEGRAM_ALLOWED_USERS": "222"}):
assert _auth_env("TELEGRAM_ALLOWED_USERS") == "222"
def test_scoped_miss_returns_default_not_env(self, monkeypatch):
from gateway.authz_mixin import _auth_env
monkeypatch.setenv("TELEGRAM_ALLOWED_USERS", "profile-A")
monkeypatch.setenv("GATEWAY_ALLOW_ALL_USERS", "true")
monkeypatch.setenv("TELEGRAM_ALLOW_ALL_USERS", "true")
ss.set_multiplex_active(True)
with _Scope({"UNRELATED": "x"}):
assert _auth_env("TELEGRAM_ALLOWED_USERS") == ""
assert _auth_env("GATEWAY_ALLOW_ALL_USERS") == ""
assert _auth_env("TELEGRAM_ALLOW_ALL_USERS") == ""
def test_single_profile_legacy_env(self, monkeypatch):
from gateway.authz_mixin import _auth_env
monkeypatch.setenv("GATEWAY_ALLOWED_USERS", "42")
assert _auth_env("GATEWAY_ALLOWED_USERS") == "42"
# ── Cluster B: matrix startup reads (Slack pattern) ────────────────────────
class TestMatrixStartupSecret:
tests/gateway/test_qqbot_scope_paths.py
+27 -10
View File
@@ -97,16 +97,6 @@ class TestAuthzAllowAllScope:
finally:
ss.reset_secret_scope(tok)
@pytest.mark.xfail(
reason=(
"gateway/authz_mixin.py still reads the platform allow-all flag via "
"_auth_env, which falls through to os.environ on a scoped miss; the "
"scope-authoritative gate (_platform_gate_env semantics) for the "
"remaining authz_mixin reads lands in a separate PR. Flips green "
"when that PR converts the allow-all read."
),
strict=True,
)
def test_scope_does_not_inherit_environ_opt_in(self, monkeypatch):
# The PRIMARY profile opted in via os.environ; the secondary profile's
# scope has no opt-in. The secondary must NOT inherit the primary's
@@ -157,6 +147,33 @@ class TestAuthzAllowlistScope:
finally:
ss.reset_secret_scope(tok)
def test_scope_miss_does_not_inherit_environ_allowlist(self, monkeypatch):
# Primary env lists the sender. Secondary scope has no allowlist
# key at all. The miss must deny, not borrow the process value.
monkeypatch.setenv("QQ_ALLOWED_USERS", "user-1")
runner = _make_qq_runner()
ss.set_multiplex_active(True)
tok = ss.set_secret_scope({})
try:
assert runner._is_user_authorized(_qq_dm_source()) is False
finally:
ss.reset_secret_scope(tok)
class TestAuthzGatewayAllowAllScope:
def test_scope_miss_does_not_inherit_gateway_allow_all(self, monkeypatch):
# GATEWAY_ALLOW_ALL_USERS is the last-chance fail-open flag, also
# read through _auth_env. A secondary profile that never set it
# must not inherit the primary's process-env opt-in.
monkeypatch.setenv("GATEWAY_ALLOW_ALL_USERS", "true")
runner = _make_qq_runner()
ss.set_multiplex_active(True)
tok = ss.set_secret_scope({})
try:
assert runner._is_user_authorized(_qq_dm_source()) is False
finally:
ss.reset_secret_scope(tok)
class TestStartupValidatorScope:
@staticmethod
tests/tools/test_bot_relay.py
+50
View File
@@ -151,6 +151,56 @@ def test_waiter_command_quotes_and_targets_reply_file(root):
assert "rm -rf" not in cmd # sanity: single quoted -c payload
def test_roster_rejects_connection_id_outside_handle_charset(root):
bad = [
{"profile": "researcher", "handle": "researcher", "connection_id": "vps'); print(1)"},
{"profile": "researcher", "handle": "researcher", "connection_id": "foo'bar"},
{"profile": "researcher", "handle": "researcher", "connection_id": "ssh vps"},
{"profile": "researcher", "handle": "researcher", "connection_id": "a" * 65},
]
assert bot_relay.write_remote_roster(root, bad) == 0
good = {
"profile": "researcher",
"handle": "researcher",
"connection_id": "ssh-vps",
}
assert bot_relay.write_remote_roster(root, [good]) == 1
def test_waiter_command_repr_encodes_hostile_connection_id(root):
import ast
import shlex
inj = "x'); open(r'/tmp/pwned','w').write('pwned'); print('x"
env = {
"id": "c" * 32,
"target_handle": "researcher",
"target_connection": inj,
}
cmd = bot_relay.waiter_command(root, env)
parts = shlex.split(cmd)
code = parts[parts.index("-c") + 1]
compile(code, "<waiter>", "exec")
tree = ast.parse(code)
opens = [
n
for n in ast.walk(tree)
if isinstance(n, ast.Call)
and isinstance(n.func, ast.Name)
and n.func.id == "open"
]
# Only json.load(open(p, ...)) is a real open(); the payload must stay data.
assert len(opens) == 1
# A quote in the id used to SyntaxError the waiter. It must compile.
quoted = bot_relay.waiter_command(
root,
{"id": "a" * 32, "target_handle": "h", "target_connection": "foo'bar"},
)
qcode = shlex.split(quoted)[shlex.split(quoted).index("-c") + 1]
compile(qcode, "<waiter-quote>", "exec")
# ── message_agent integration: relay route + legacy-SOUL gate fix ───────────
import textwrap
tools/bot_relay.py
+15 -5
View File
@@ -124,7 +124,11 @@ def _normalize_roster_row(row: Any) -> Optional[dict]:
return None
if not handle:
handle = "hermes" if profile == "default" else profile
if not _HANDLE_RE.match(handle) or not _HANDLE_RE.match(profile):
if (
not _HANDLE_RE.match(handle)
or not _HANDLE_RE.match(profile)
or not _HANDLE_RE.match(connection_id)
):
return None
out = {
"profile": profile,
@@ -486,22 +490,28 @@ def waiter_command(root: Path | str, envelope: dict) -> str:
interpreter.
"""
reply_path = str(relay_root(root) / REPLIES_DIR / f"{envelope['id']}.json")
label = f"@{envelope['target_handle']} on {envelope['target_connection']}"
label = (
f"@{envelope.get('target_handle', '')} "
f"on {envelope.get('target_connection', '')}"
)
# Encode label with !r so roster fields cannot break out of the generated
# python -c source (quotes, parens, or extra statements in connection_id).
code = (
"import json,os,sys,time\n"
f"p = {reply_path!r}\n"
f"label = {label!r}\n"
f"deadline = time.time() + {REPLY_WAIT_SECONDS}\n"
"while time.time() < deadline:\n"
" if os.path.exists(p):\n"
" d = json.load(open(p, encoding='utf-8'))\n"
" if d.get('error'):\n"
f" print('Delivery to {label} failed: ' + d['error'])\n"
" print('Delivery to ' + label + ' failed: ' + d['error'])\n"
" sys.exit(1)\n"
f" print('Reply from {label}:')\n"
" print('Reply from ' + label + ':')\n"
" print(d.get('reply') or '(empty reply)')\n"
" sys.exit(0)\n"
" time.sleep(2)\n"
f"print('No reply from {label} within {REPLY_WAIT_SECONDS}s. The message may "
f"print('No reply from ' + label + ' within {REPLY_WAIT_SECONDS}s. The message may "
"still be delivered when the Desktop reconnects; do not resend blindly.')\n"
"sys.exit(1)\n"
)