mirror of
https://github.com/MervinPraison/PraisonAI.git
synced 2026-11-07 04:35:58 -10:00
fix: doctor gateway security check respects unknown_user_policy (#6046)
* fix: make doctor gateway security check respect unknown_user_policy (fixes #5092) The gateway_security doctor check reported 'bot responds to EVERYONE!' for any channel without an allowlist, even under the default unknown_user_policy=deny where unknown DMs are dropped. This is the same misleading text #5112 removed from the startup banner. Flag open access only for policy=allow; warn that DMs are dropped for deny. Co-Authored-By: Claude Opus 5.5 <[email protected]> * fix: doctor treats any non-pair unknown_user_policy as deny, matching runtime Co-Authored-By: Claude Opus 5.5 <[email protected]> --------- Co-authored-by: MervinPraison <[email protected]> Co-authored-by: Claude Opus 5.5 <[email protected]>
This commit is contained in:
2 files changed
+65
-4
No files matched your search
@@ -151,11 +151,19 @@ def check_gateway_security(config: DoctorConfig) -> CheckResult:
|
||||
warnings = []
|
||||
|
||||
for channel_name, channel in validated_config.channels.items():
|
||||
# Check for open allowlists (CRITICAL)
|
||||
# Check for open allowlists. Only unknown_user_policy=allow opens
|
||||
# the bot to everyone; the default deny drops unknown DMs (Issue #5092).
|
||||
if not channel.allowed_users and not channel.allowlist and not channel.blocklist:
|
||||
security_issues.append(
|
||||
f"{channel_name}: No user restrictions - bot responds to EVERYONE!"
|
||||
)
|
||||
policy = (channel.unknown_user_policy or "deny").lower()
|
||||
if policy == "allow":
|
||||
security_issues.append(
|
||||
f"{channel_name}: No user restrictions - bot responds to EVERYONE!"
|
||||
)
|
||||
elif policy != "pair": # deny (default), mirrors runtime fallback
|
||||
warnings.append(
|
||||
f"{channel_name}: No allowed_users and unknown_user_policy=deny "
|
||||
"- DMs from unknown users are dropped"
|
||||
)
|
||||
|
||||
# Check for overly permissive group policies (WARNING)
|
||||
if channel.group_policy == "respond_all":
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
"""The gateway security check must reflect ``unknown_user_policy`` (Issue #5092).
|
||||
|
||||
An empty allowlist only opens the bot to everyone when the policy is
|
||||
``allow``; under the default ``deny`` unknown DMs are dropped.
|
||||
"""
|
||||
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from praisonai_code.cli.features.doctor.models import CheckStatus, DoctorConfig
|
||||
from praisonai_code.cli.features.doctor.checks import gateway_checks
|
||||
|
||||
|
||||
def _run(monkeypatch, tmp_path, policy):
|
||||
(tmp_path / "gateway.yaml").write_text("channels: {}\n")
|
||||
monkeypatch.chdir(tmp_path)
|
||||
monkeypatch.setenv("GATEWAY_AUTH_TOKEN", "x")
|
||||
monkeypatch.setattr(gateway_checks, "skip_if_no_wrapper", lambda *a, **k: None)
|
||||
monkeypatch.setattr(gateway_checks, "skip_if_no_bot_package", lambda *a, **k: None)
|
||||
monkeypatch.setattr(
|
||||
"praisonai_code.cli._paths.resolve_bot_config_path", lambda name: name
|
||||
)
|
||||
channel = SimpleNamespace(
|
||||
allowed_users=[], allowlist=[], blocklist=[],
|
||||
unknown_user_policy=policy, group_policy="mention_only", token="t",
|
||||
)
|
||||
schema = SimpleNamespace(
|
||||
load_and_validate_gateway_yaml=lambda path: SimpleNamespace(
|
||||
channels={"telegram": channel}
|
||||
)
|
||||
)
|
||||
monkeypatch.setattr(gateway_checks, "_bots_config_schema", lambda: schema)
|
||||
return gateway_checks.check_gateway_security(DoctorConfig())
|
||||
|
||||
|
||||
def test_allow_policy_flags_open_access(monkeypatch, tmp_path):
|
||||
result = _run(monkeypatch, tmp_path, "allow")
|
||||
assert result.status == CheckStatus.FAIL
|
||||
assert "EVERYONE" in result.details
|
||||
|
||||
|
||||
@pytest.mark.parametrize("policy", [None, "deny"])
|
||||
def test_deny_policy_does_not_claim_everyone(monkeypatch, tmp_path, policy):
|
||||
result = _run(monkeypatch, tmp_path, policy)
|
||||
assert result.status == CheckStatus.WARN
|
||||
assert "EVERYONE" not in result.details
|
||||
assert "dropped" in result.details
|
||||
|
||||
|
||||
def test_pair_policy_passes(monkeypatch, tmp_path):
|
||||
result = _run(monkeypatch, tmp_path, "pair")
|
||||
assert result.status == CheckStatus.PASS
|
||||
Reference in new issue
Block a user