fix(email): Slack-pattern helper for unscoped default-profile adapter + scope ports/trust flag
Follow-up to the salvaged #59076 commit:
- Replace the bare get_secret import with a module-level Slack-pattern
helper (_get_esecret): try get_secret, on UnscopedSecretError fall back
to os.getenv. The DEFAULT profile's email adapter constructs UNSCOPED
under multiplexing, where a bare get_secret raises and would crash the
email path on startup — the exact WhatsApp defect fixed in 5438e9c629
(whatsapp_common._get_wsecret).
- Extend scope coverage to the remaining scope-blind reads:
EMAIL_IMAP_PORT / EMAIL_SMTP_PORT / EMAIL_POLL_INTERVAL (_esecret_int
replacing utils.env_int) and EMAIL_TRUST_FROM_HEADER (_esecret_bool
replacing utils.env_bool).
- Add tests: default-profile unscoped-under-multiplex construction, and
scoped ports/trust-flag no-environ-inheritance.
This commit is contained in:
parent
f08f403157
commit
ff89f1b862
|
|
@ -25,7 +25,8 @@ import smtplib
|
|||
import socket
|
||||
|
||||
# Profile-scoped secret reader for multiplexing support (PR #50094)
|
||||
from agent.secret_scope import get_secret as _get_secret
|
||||
from agent.secret_scope import UnscopedSecretError as _UnscopedSecretError
|
||||
from agent.secret_scope import get_secret as _scoped_get_secret
|
||||
import ssl
|
||||
import uuid
|
||||
from email.header import decode_header
|
||||
|
|
@ -46,9 +47,50 @@ from gateway.platforms.base import (
|
|||
cache_image_from_bytes,
|
||||
)
|
||||
from gateway.config import Platform, PlatformConfig
|
||||
from utils import env_int, env_bool
|
||||
from utils import is_truthy_value
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def _get_esecret(name: str, default: str = "") -> str:
|
||||
"""Scope-aware ``EMAIL_*`` read with the default-profile startup fallback.
|
||||
|
||||
Secondary profiles run under ``_profile_runtime_scope`` — the scope is
|
||||
authoritative and a scoped miss returns ``default`` (no cross-profile
|
||||
borrow). The DEFAULT profile's adapter constructs and sends *unscoped*
|
||||
under multiplexing, where a bare ``get_secret`` would raise
|
||||
``UnscopedSecretError`` and crash its email path; there ``os.environ``
|
||||
is that profile's own value, so fall back to it. Same pattern as the
|
||||
Slack ``SLACK_APP_TOKEN`` read (#59739) and the WhatsApp
|
||||
``_get_wsecret`` fix (5438e9c629).
|
||||
"""
|
||||
try:
|
||||
val = _scoped_get_secret(name, default)
|
||||
except _UnscopedSecretError:
|
||||
val = os.getenv(name)
|
||||
return val if val is not None else default
|
||||
|
||||
|
||||
# Backwards-compatible alias for the name used by the original #59076 hunks.
|
||||
_get_secret = _get_esecret
|
||||
|
||||
|
||||
def _esecret_int(name: str, default: int) -> int:
|
||||
"""Scope-aware integer read (``env_int`` variant of ``_get_esecret``)."""
|
||||
raw = str(_get_esecret(name, "")).strip()
|
||||
if not raw:
|
||||
return default
|
||||
try:
|
||||
return int(raw)
|
||||
except (ValueError, TypeError):
|
||||
return default
|
||||
|
||||
|
||||
def _esecret_bool(name: str, default: bool = False) -> bool:
|
||||
"""Scope-aware boolean read (``env_bool`` variant of ``_get_esecret``)."""
|
||||
return is_truthy_value(_get_esecret(name, ""), default=default)
|
||||
|
||||
|
||||
# Automated sender patterns — emails from these are silently ignored
|
||||
_NOREPLY_PATTERNS = (
|
||||
"noreply", "no-reply", "no_reply", "donotreply", "do-not-reply",
|
||||
|
|
@ -440,10 +482,10 @@ class EmailAdapter(BasePlatformAdapter):
|
|||
self._address = (_get_secret("EMAIL_ADDRESS", "") or extra.get("address", "")).strip()
|
||||
self._password = _get_secret("EMAIL_PASSWORD", "")
|
||||
self._imap_host = (_get_secret("EMAIL_IMAP_HOST", "") or extra.get("imap_host", "")).strip()
|
||||
self._imap_port = env_int("EMAIL_IMAP_PORT", 993)
|
||||
self._imap_port = _esecret_int("EMAIL_IMAP_PORT", 993)
|
||||
self._smtp_host = (_get_secret("EMAIL_SMTP_HOST", "") or extra.get("smtp_host", "")).strip()
|
||||
self._smtp_port = env_int("EMAIL_SMTP_PORT", 587)
|
||||
self._poll_interval = env_int("EMAIL_POLL_INTERVAL", 15)
|
||||
self._smtp_port = _esecret_int("EMAIL_SMTP_PORT", 587)
|
||||
self._poll_interval = _esecret_int("EMAIL_POLL_INTERVAL", 15)
|
||||
|
||||
# Skip attachments — configured via config.yaml:
|
||||
# platforms:
|
||||
|
|
@ -467,7 +509,7 @@ class EmailAdapter(BasePlatformAdapter):
|
|||
# gate below is skipped.
|
||||
if "require_authenticated_sender" in extra:
|
||||
self._require_authenticated_sender = bool(extra["require_authenticated_sender"])
|
||||
elif env_bool("EMAIL_TRUST_FROM_HEADER", False):
|
||||
elif _esecret_bool("EMAIL_TRUST_FROM_HEADER", False):
|
||||
self._require_authenticated_sender = False
|
||||
else:
|
||||
self._require_authenticated_sender = True
|
||||
|
|
|
|||
|
|
@ -157,5 +157,92 @@ class TestEmailAdapterSecretScope(unittest.TestCase):
|
|||
ss.reset_secret_scope(token)
|
||||
|
||||
|
||||
class TestEmailAdapterUnscopedUnderMultiplex(unittest.TestCase):
|
||||
"""The DEFAULT profile's adapter constructs UNSCOPED under multiplexing.
|
||||
|
||||
In a multiplexed gateway only secondary profiles run inside
|
||||
``_profile_runtime_scope``; the default profile's adapter is constructed
|
||||
with no scope installed while multiplexing is active. A bare
|
||||
``get_secret`` raises ``UnscopedSecretError`` there and would crash the
|
||||
email path on startup — the exact WhatsApp defect fixed in 5438e9c629.
|
||||
The Slack-pattern helper (``_get_esecret``) must swallow that and read
|
||||
the default profile's own ``os.environ`` values instead.
|
||||
"""
|
||||
|
||||
def setUp(self):
|
||||
ss.set_multiplex_active(False)
|
||||
|
||||
def tearDown(self):
|
||||
ss.set_multiplex_active(False)
|
||||
|
||||
@patch.dict(os.environ, {
|
||||
"EMAIL_ADDRESS": "alpha@test.invalid",
|
||||
"EMAIL_PASSWORD": "default-pw",
|
||||
"EMAIL_IMAP_HOST": "imap.default.com",
|
||||
"EMAIL_SMTP_HOST": "smtp.default.com",
|
||||
"EMAIL_IMAP_PORT": "1993",
|
||||
"EMAIL_SMTP_PORT": "1587",
|
||||
"EMAIL_POLL_INTERVAL": "30",
|
||||
}, clear=False)
|
||||
def test_default_profile_constructs_unscoped_under_multiplex(self):
|
||||
"""Multiplex ON + no scope: construction must not raise and must
|
||||
read the default profile's own environ values."""
|
||||
from gateway.config import PlatformConfig
|
||||
from plugins.platforms.email.adapter import EmailAdapter
|
||||
|
||||
ss.set_multiplex_active(True)
|
||||
cfg = PlatformConfig(enabled=True)
|
||||
adapter = EmailAdapter(cfg) # must NOT raise UnscopedSecretError
|
||||
self.assertEqual(adapter._address, "alpha@test.invalid")
|
||||
self.assertEqual(adapter._password, "default-pw")
|
||||
self.assertEqual(adapter._imap_host, "imap.default.com")
|
||||
self.assertEqual(adapter._smtp_host, "smtp.default.com")
|
||||
self.assertEqual(adapter._imap_port, 1993)
|
||||
self.assertEqual(adapter._smtp_port, 1587)
|
||||
self.assertEqual(adapter._poll_interval, 30)
|
||||
|
||||
@patch.dict(os.environ, {
|
||||
"EMAIL_ADDRESS": "alpha@test.invalid",
|
||||
"EMAIL_PASSWORD": "default-pw",
|
||||
"EMAIL_IMAP_HOST": "imap.default.com",
|
||||
"EMAIL_SMTP_HOST": "smtp.default.com",
|
||||
"EMAIL_IMAP_PORT": "1993",
|
||||
"EMAIL_SMTP_PORT": "1587",
|
||||
"EMAIL_POLL_INTERVAL": "30",
|
||||
"EMAIL_TRUST_FROM_HEADER": "true",
|
||||
}, clear=False)
|
||||
def test_scoped_ports_and_trust_flag_do_not_inherit_environ(self):
|
||||
"""The env_int variants (EMAIL_IMAP_PORT / EMAIL_SMTP_PORT /
|
||||
EMAIL_POLL_INTERVAL) and EMAIL_TRUST_FROM_HEADER must resolve from
|
||||
the installed scope, not from the primary profile's environ."""
|
||||
from gateway.config import PlatformConfig
|
||||
from plugins.platforms.email.adapter import EmailAdapter
|
||||
|
||||
scoped = {
|
||||
"EMAIL_ADDRESS": "beta@test.invalid",
|
||||
"EMAIL_PASSWORD": "secondary-pw",
|
||||
"EMAIL_IMAP_HOST": "imap.secondary.example",
|
||||
"EMAIL_SMTP_HOST": "smtp.secondary.example",
|
||||
"EMAIL_IMAP_PORT": "2993",
|
||||
"EMAIL_SMTP_PORT": "2587",
|
||||
"EMAIL_POLL_INTERVAL": "60",
|
||||
# scope does NOT opt into trusting From: — environ's "true"
|
||||
# must not leak in.
|
||||
}
|
||||
ss.set_multiplex_active(True)
|
||||
token = ss.set_secret_scope(scoped)
|
||||
try:
|
||||
cfg = PlatformConfig(enabled=True)
|
||||
adapter = EmailAdapter(cfg)
|
||||
self.assertEqual(adapter._imap_port, 2993)
|
||||
self.assertEqual(adapter._smtp_port, 2587)
|
||||
self.assertEqual(adapter._poll_interval, 60)
|
||||
# Environ's EMAIL_TRUST_FROM_HEADER=true must not leak in: the
|
||||
# scope did not opt out, so authentication stays required.
|
||||
self.assertTrue(adapter._require_authenticated_sender)
|
||||
finally:
|
||||
ss.reset_secret_scope(token)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
|
|
|||
Loading…
Reference in New Issue