fix(gateway): repair sibling tests + harden _adapter_for_source after fail-closed flip
Follow-up to the salvaged fail-closed defaults. The own-policy default flip (open -> pairing) and the email dispatch-level deny broke sibling tests across the suite that relied on the old fail-open behavior: - test_email.py: dispatch-mechanics tests now opt into EMAIL_ALLOW_ALL_USERS (they test formatting/attachments/threading, not authz); the two auth contract tests are rewritten to assert the new fail-closed behavior (no allowlist + no allow-all => sender dropped at the adapter). - test_whatsapp_cloud.py / test_whatsapp_formatting.py / test_whatsapp_from_owner.py: autouse fixture opts into WHATSAPP_ALLOW_ALL_USERS so dm_policy: open dispatch-mechanics tests still flow (open now requires an explicit allow-all opt-in, SECURITY.md 2.6). - _adapter_for_source: use getattr for source.platform/profile so bare SimpleNamespace test fixtures without .profile don't crash the busy/queue ingress path (AGENTS.md pitfall #17). Full tests/gateway/ + yuanbao pipeline: 8555 passed, 0 failed.
This commit is contained in:
parent
49a87bcd1e
commit
d1d1d81900
5 changed files with 109 additions and 12 deletions
|
|
@ -58,7 +58,12 @@ class GatewayAuthorizationMixin:
|
|||
"""Resolve the live adapter for an inbound ``SessionSource``."""
|
||||
if source is None:
|
||||
return None
|
||||
return self._authorization_adapter(source.platform, source.profile)
|
||||
# ``getattr`` guards test fixtures that build a bare source via
|
||||
# SimpleNamespace and omit ``profile`` (see AGENTS.md pitfall #17).
|
||||
return self._authorization_adapter(
|
||||
getattr(source, "platform", None),
|
||||
getattr(source, "profile", None),
|
||||
)
|
||||
|
||||
def _adapter_authorization_is_upstream(
|
||||
self,
|
||||
|
|
|
|||
|
|
@ -236,6 +236,21 @@ class TestExtractAttachments(unittest.TestCase):
|
|||
class TestDispatchMessage(unittest.TestCase):
|
||||
"""Test email message dispatch logic."""
|
||||
|
||||
def setUp(self):
|
||||
# These tests exercise dispatch mechanics (subject formatting,
|
||||
# attachment typing, source building), not the authorization gate.
|
||||
# The adapter now fails closed at dispatch when no allowlist / allow-all
|
||||
# is configured (SECURITY.md 2.6), so opt into allow-all here to keep
|
||||
# exercising the dispatch path. Auth-contract tests below override this.
|
||||
self._prev_allow_all = os.environ.get("EMAIL_ALLOW_ALL_USERS")
|
||||
os.environ["EMAIL_ALLOW_ALL_USERS"] = "true"
|
||||
|
||||
def tearDown(self):
|
||||
if self._prev_allow_all is None:
|
||||
os.environ.pop("EMAIL_ALLOW_ALL_USERS", None)
|
||||
else:
|
||||
os.environ["EMAIL_ALLOW_ALL_USERS"] = self._prev_allow_all
|
||||
|
||||
def _make_adapter(self):
|
||||
"""Create an EmailAdapter with mocked env vars."""
|
||||
from gateway.config import PlatformConfig
|
||||
|
|
@ -547,13 +562,14 @@ class TestDispatchMessage(unittest.TestCase):
|
|||
self.assertEqual(len(captured_events), 1)
|
||||
self.assertEqual(captured_events[0].source.chat_id, "admin@test.com")
|
||||
|
||||
def test_empty_allowlist_allows_all(self):
|
||||
"""When EMAIL_ALLOWED_USERS is not set, all senders should proceed."""
|
||||
def test_empty_allowlist_denies_without_optin(self):
|
||||
"""No allowlist and no allow-all opt-in → adapter fails closed (2.6)."""
|
||||
import asyncio
|
||||
with patch.dict(os.environ, {}, clear=False):
|
||||
# Ensure EMAIL_ALLOWED_USERS is not in the env
|
||||
if "EMAIL_ALLOWED_USERS" in os.environ:
|
||||
del os.environ["EMAIL_ALLOWED_USERS"]
|
||||
# No allowlist, and explicitly no allow-all opt-in.
|
||||
for k in ("EMAIL_ALLOWED_USERS", "EMAIL_ALLOW_ALL_USERS",
|
||||
"GATEWAY_ALLOW_ALL_USERS"):
|
||||
os.environ.pop(k, None)
|
||||
|
||||
adapter = self._make_adapter()
|
||||
adapter._message_handler = MagicMock()
|
||||
|
|
@ -571,7 +587,32 @@ class TestDispatchMessage(unittest.TestCase):
|
|||
}
|
||||
|
||||
asyncio.run(adapter._dispatch_message(msg_data))
|
||||
# Handler should be called when no allowlist is configured
|
||||
# Fail closed: an unset allowlist without allow-all drops the sender.
|
||||
adapter._message_handler.assert_not_called()
|
||||
|
||||
def test_empty_allowlist_allows_all_with_optin(self):
|
||||
"""EMAIL_ALLOW_ALL_USERS=true with no allowlist → all senders proceed."""
|
||||
import asyncio
|
||||
with patch.dict(os.environ, {"EMAIL_ALLOW_ALL_USERS": "true"}, clear=False):
|
||||
os.environ.pop("EMAIL_ALLOWED_USERS", None)
|
||||
|
||||
adapter = self._make_adapter()
|
||||
adapter._message_handler = MagicMock()
|
||||
|
||||
msg_data = {
|
||||
"uid": b"101",
|
||||
"sender_addr": "anyone@test.com",
|
||||
"sender_name": "Anyone",
|
||||
"subject": "Hey",
|
||||
"message_id": "<any@test.com>",
|
||||
"in_reply_to": "",
|
||||
"body": "Hi",
|
||||
"attachments": [],
|
||||
"date": "",
|
||||
}
|
||||
|
||||
asyncio.run(adapter._dispatch_message(msg_data))
|
||||
# With explicit allow-all opt-in the handler is called.
|
||||
adapter._message_handler.assert_called()
|
||||
|
||||
def test_spoofed_from_rejected_when_allowlisted(self):
|
||||
|
|
@ -584,6 +625,8 @@ class TestDispatchMessage(unittest.TestCase):
|
|||
import asyncio
|
||||
with patch.dict(os.environ, {
|
||||
"EMAIL_ALLOWED_USERS": "admin@test.com",
|
||||
"EMAIL_ALLOW_ALL_USERS": "",
|
||||
"GATEWAY_ALLOW_ALL_USERS": "",
|
||||
}):
|
||||
adapter = self._make_adapter()
|
||||
adapter._message_handler = MagicMock()
|
||||
|
|
@ -606,11 +649,12 @@ class TestDispatchMessage(unittest.TestCase):
|
|||
adapter._message_handler.assert_not_called()
|
||||
self.assertNotIn("admin@test.com", adapter._thread_context)
|
||||
|
||||
def test_unauthenticated_allowed_without_allowlist(self):
|
||||
"""No allowlist → no From: auth gate (gateway default-denies anyway)."""
|
||||
def test_unauthenticated_denied_without_allowlist_optin(self):
|
||||
"""No allowlist, no allow-all → adapter fails closed regardless of From auth."""
|
||||
import asyncio
|
||||
with patch.dict(os.environ, {}, clear=False):
|
||||
for k in ("EMAIL_ALLOWED_USERS", "GATEWAY_ALLOWED_USERS"):
|
||||
for k in ("EMAIL_ALLOWED_USERS", "GATEWAY_ALLOWED_USERS",
|
||||
"EMAIL_ALLOW_ALL_USERS", "GATEWAY_ALLOW_ALL_USERS"):
|
||||
os.environ.pop(k, None)
|
||||
adapter = self._make_adapter()
|
||||
adapter._message_handler = MagicMock()
|
||||
|
|
@ -630,8 +674,8 @@ class TestDispatchMessage(unittest.TestCase):
|
|||
}
|
||||
|
||||
asyncio.run(adapter._dispatch_message(msg_data))
|
||||
# Adapter forwards; the gateway's own default-deny handles authz.
|
||||
adapter._message_handler.assert_called()
|
||||
# Fail closed at the adapter — no allowlist and no allow-all opt-in.
|
||||
adapter._message_handler.assert_not_called()
|
||||
|
||||
def test_unauthenticated_allowed_with_trust_from_header(self):
|
||||
"""EMAIL_TRUST_FROM_HEADER=true disables the gate even with an allowlist."""
|
||||
|
|
@ -706,6 +750,19 @@ class TestDispatchMessage(unittest.TestCase):
|
|||
class TestThreadContext(unittest.TestCase):
|
||||
"""Test email reply threading logic."""
|
||||
|
||||
def setUp(self):
|
||||
# Thread-context storage is a dispatch-mechanics test, not an auth test.
|
||||
# The adapter fails closed at dispatch without allow-all (SECURITY.md 2.6),
|
||||
# so opt into allow-all to keep exercising the threading path.
|
||||
self._prev_allow_all = os.environ.get("EMAIL_ALLOW_ALL_USERS")
|
||||
os.environ["EMAIL_ALLOW_ALL_USERS"] = "true"
|
||||
|
||||
def tearDown(self):
|
||||
if self._prev_allow_all is None:
|
||||
os.environ.pop("EMAIL_ALLOW_ALL_USERS", None)
|
||||
else:
|
||||
os.environ["EMAIL_ALLOW_ALL_USERS"] = self._prev_allow_all
|
||||
|
||||
def _make_adapter(self):
|
||||
from gateway.config import PlatformConfig
|
||||
with patch.dict(os.environ, {
|
||||
|
|
|
|||
|
|
@ -20,6 +20,20 @@ import pytest
|
|||
from gateway.config import Platform
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _whatsapp_open_optin(monkeypatch):
|
||||
"""Opt into WhatsApp allow-all for the file's dispatch-mechanics tests.
|
||||
|
||||
The adapter now fails closed on ``dm_policy: open`` unless
|
||||
``WHATSAPP_ALLOW_ALL_USERS`` / ``GATEWAY_ALLOW_ALL_USERS`` is set
|
||||
(SECURITY.md 2.6). These tests set ``_dm_policy = "open"`` as a stand-in
|
||||
for "process this DM" while exercising unrelated dispatch mechanics, so
|
||||
grant the opt-in here. Tests that specifically assert the gate override
|
||||
this within their own body.
|
||||
"""
|
||||
monkeypatch.setenv("WHATSAPP_ALLOW_ALL_USERS", "true")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -14,6 +14,17 @@ import pytest
|
|||
from gateway.config import Platform
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _whatsapp_open_optin(monkeypatch):
|
||||
"""Opt into WhatsApp allow-all so ``dm_policy: open`` dispatch tests run.
|
||||
|
||||
The adapter fails closed on ``open`` without an allow-all opt-in
|
||||
(SECURITY.md 2.6); these formatting/dispatch-mechanics tests set
|
||||
``_dm_policy = "open"`` as a stand-in for "process this DM".
|
||||
"""
|
||||
monkeypatch.setenv("WHATSAPP_ALLOW_ALL_USERS", "true")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -21,6 +21,16 @@ from gateway.config import Platform, PlatformConfig
|
|||
from plugins.platforms.whatsapp.adapter import WhatsAppAdapter
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _whatsapp_open_optin(monkeypatch):
|
||||
"""Opt into WhatsApp allow-all so ``dm_policy: open`` dispatch tests run.
|
||||
|
||||
The adapter fails closed on ``open`` without an allow-all opt-in
|
||||
(SECURITY.md 2.6); these owner-DM tests set ``_dm_policy = "open"``.
|
||||
"""
|
||||
monkeypatch.setenv("WHATSAPP_ALLOW_ALL_USERS", "true")
|
||||
|
||||
|
||||
def _make_adapter():
|
||||
adapter = WhatsAppAdapter.__new__(WhatsAppAdapter)
|
||||
adapter.platform = Platform.WHATSAPP
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue