fix(email): honor profile secret scope for email adapter env reads
The email adapter (plugins/platforms/email/adapter.py) read EMAIL_ADDRESS, EMAIL_PASSWORD, EMAIL_IMAP_HOST, EMAIL_SMTP_HOST, EMAIL_ALLOWED_USERS, and EMAIL_ALLOW_ALL_USERS via os.getenv() directly. In a multiplexed gateway, os.environ holds the default profile's .env values, so every secondary profile inherited the default profile's email credentials instead of its own. This was a sibling of the api_server env-leak bug (#52307/#50051): the same os.getenv→get_secret migration that PR #50094 applies to gateway/config.py, but for the email adapter itself, which neither PR #50094 nor #51374 covers. Changes: - plugins/platforms/email/adapter.py: replace os.getenv with agent.secret_scope.get_secret for all EMAIL_* credential reads (adapter __init__, check_email_requirements, _allowlist_in_effect, _dispatch_message allowlist gate, _send_email SMTP helper). - gateway/config.py: add _getenv/_getenv_str/_getenv_int helpers (from PR #50094) and replace os.getenv with _getenv for the email block in _apply_env_overrides, so config.platforms[EMAIL].extra is populated from the scoped value. - tests/gateway/test_email_secret_scope.py: 5 new tests covering scoped credential reads, environ fallback without scope, missing- key-no-leak, allowlist scoping, and check_email_requirements scoping. Related: #50051, #52307, PR #50094, PR #51374
This commit is contained in:
shikanga-hermes
Teknium
parent
ed9986873d
commit
f08f403157
|
|
@ -23,6 +23,9 @@ import os
|
|||
import re
|
||||
import smtplib
|
||||
import socket
|
||||
|
||||
# Profile-scoped secret reader for multiplexing support (PR #50094)
|
||||
from agent.secret_scope import get_secret as _get_secret
|
||||
import ssl
|
||||
import uuid
|
||||
from email.header import decode_header
|
||||
|
|
@ -164,10 +167,10 @@ def check_email_requirements() -> bool:
|
|||
Treats blank/whitespace-only values as missing so an abandoned setup that
|
||||
left empty ``EMAIL_*`` keys in ``.env`` does not enable the platform (#40715).
|
||||
"""
|
||||
addr = os.getenv("EMAIL_ADDRESS", "").strip()
|
||||
pwd = os.getenv("EMAIL_PASSWORD", "").strip()
|
||||
imap = os.getenv("EMAIL_IMAP_HOST", "").strip()
|
||||
smtp = os.getenv("EMAIL_SMTP_HOST", "").strip()
|
||||
addr = _get_secret("EMAIL_ADDRESS", "").strip()
|
||||
pwd = _get_secret("EMAIL_PASSWORD", "").strip()
|
||||
imap = _get_secret("EMAIL_IMAP_HOST", "").strip()
|
||||
smtp = _get_secret("EMAIL_SMTP_HOST", "").strip()
|
||||
return all([addr, pwd, imap, smtp])
|
||||
|
||||
|
||||
|
|
@ -434,11 +437,11 @@ class EmailAdapter(BasePlatformAdapter):
|
|||
# misleading ``[Errno 8] nodename nor servname`` (an unresolvable name)
|
||||
# instead of an obvious "host not set" error.
|
||||
extra = config.extra or {}
|
||||
self._address = (os.getenv("EMAIL_ADDRESS", "") or extra.get("address", "")).strip()
|
||||
self._password = os.getenv("EMAIL_PASSWORD", "")
|
||||
self._imap_host = (os.getenv("EMAIL_IMAP_HOST", "") or extra.get("imap_host", "")).strip()
|
||||
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._smtp_host = (os.getenv("EMAIL_SMTP_HOST", "") or extra.get("smtp_host", "")).strip()
|
||||
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)
|
||||
|
||||
|
|
@ -473,7 +476,7 @@ class EmailAdapter(BasePlatformAdapter):
|
|||
# own receiving server (defends against an injected header that sorts
|
||||
# first). Defaults to the From-domain of the agent's own address.
|
||||
self._authserv_id = (
|
||||
extra.get("authserv_id", "") or os.getenv("EMAIL_AUTHSERV_ID", "")
|
||||
extra.get("authserv_id", "") or _get_secret("EMAIL_AUTHSERV_ID", "")
|
||||
).strip().lower()
|
||||
|
||||
# Track message IDs we've already processed to avoid duplicates
|
||||
|
|
@ -756,7 +759,7 @@ class EmailAdapter(BasePlatformAdapter):
|
|||
"""
|
||||
truthy = {"true", "1", "yes"}
|
||||
return (
|
||||
os.getenv("EMAIL_ALLOW_ALL_USERS", "").strip().lower() in truthy
|
||||
_get_secret("EMAIL_ALLOW_ALL_USERS", "").strip().lower() in truthy
|
||||
or os.getenv("GATEWAY_ALLOW_ALL_USERS", "").strip().lower() in truthy
|
||||
)
|
||||
|
||||
|
|
@ -771,7 +774,7 @@ class EmailAdapter(BasePlatformAdapter):
|
|||
and the authentication gate is unnecessary.
|
||||
"""
|
||||
return bool(
|
||||
os.getenv("EMAIL_ALLOWED_USERS", "").strip()
|
||||
_get_secret("EMAIL_ALLOWED_USERS", "").strip()
|
||||
or os.getenv("GATEWAY_ALLOWED_USERS", "").strip()
|
||||
)
|
||||
|
||||
|
|
@ -793,9 +796,9 @@ class EmailAdapter(BasePlatformAdapter):
|
|||
# that the gateway will never authorize. Without this early guard,
|
||||
# a race between dispatch and authorization can result in the adapter
|
||||
# sending a reply even though the handler returned None.
|
||||
allowed_raw = os.getenv("EMAIL_ALLOWED_USERS", "").strip()
|
||||
allowed_raw = _get_secret("EMAIL_ALLOWED_USERS", "").strip()
|
||||
if not allowed_raw:
|
||||
if os.getenv("EMAIL_ALLOW_ALL_USERS", "").strip().lower() not in {"true", "1", "yes"} and (
|
||||
if _get_secret("EMAIL_ALLOW_ALL_USERS", "").strip().lower() not in {"true", "1", "yes"} and (
|
||||
os.getenv("GATEWAY_ALLOW_ALL_USERS", "").strip().lower() not in {"true", "1", "yes"}
|
||||
):
|
||||
logger.debug(
|
||||
|
|
@ -1204,11 +1207,11 @@ async def _standalone_send(
|
|||
from email.utils import formatdate
|
||||
|
||||
extra = getattr(pconfig, "extra", {}) or {}
|
||||
address = extra.get("address") or os.getenv("EMAIL_ADDRESS", "")
|
||||
password = os.getenv("EMAIL_PASSWORD", "")
|
||||
smtp_host = extra.get("smtp_host") or os.getenv("EMAIL_SMTP_HOST", "")
|
||||
address = extra.get("address") or _get_secret("EMAIL_ADDRESS", "")
|
||||
password = _get_secret("EMAIL_PASSWORD", "")
|
||||
smtp_host = extra.get("smtp_host") or _get_secret("EMAIL_SMTP_HOST", "")
|
||||
try:
|
||||
smtp_port = int(os.getenv("EMAIL_SMTP_PORT", "587"))
|
||||
smtp_port = int(_get_secret("EMAIL_SMTP_PORT", "587") or "587")
|
||||
except (ValueError, TypeError):
|
||||
smtp_port = 587
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,161 @@
|
|||
"""Tests for email adapter credential isolation under multiplexing.
|
||||
|
||||
Verifies that the email adapter reads EMAIL_ADDRESS, EMAIL_PASSWORD,
|
||||
EMAIL_IMAP_HOST, and EMAIL_SMTP_HOST from the profile-scoped secret
|
||||
store (agent.secret_scope.get_secret) instead of os.getenv, so that
|
||||
a secondary profile in a multiplexed gateway does not inherit the
|
||||
default profile's email credentials via os.environ.
|
||||
|
||||
Related issues: #50051, #52307
|
||||
Related PRs: #51374 (config.py api_server guard), #50094 (config.py scoped env reads)
|
||||
"""
|
||||
|
||||
import os
|
||||
import unittest
|
||||
from unittest.mock import patch, MagicMock
|
||||
|
||||
from agent import secret_scope as ss
|
||||
|
||||
|
||||
class TestEmailAdapterSecretScope(unittest.TestCase):
|
||||
"""Verify the email adapter honors the profile secret scope over os.environ."""
|
||||
|
||||
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",
|
||||
}, clear=False)
|
||||
def test_adapter_uses_scoped_credentials_not_environ(self):
|
||||
"""When a secret scope is installed, the adapter must read from it,
|
||||
not from os.environ which may hold another profile's values."""
|
||||
from gateway.config import PlatformConfig, Platform
|
||||
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",
|
||||
}
|
||||
ss.set_multiplex_active(True)
|
||||
token = ss.set_secret_scope(scoped)
|
||||
try:
|
||||
cfg = PlatformConfig(enabled=True)
|
||||
adapter = EmailAdapter(cfg)
|
||||
self.assertEqual(adapter._address, "beta@test.invalid")
|
||||
self.assertEqual(adapter._password, "secondary-pw")
|
||||
self.assertEqual(adapter._imap_host, "imap.secondary.example")
|
||||
self.assertEqual(adapter._smtp_host, "smtp.secondary.example")
|
||||
finally:
|
||||
ss.reset_secret_scope(token)
|
||||
|
||||
@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",
|
||||
}, clear=False)
|
||||
def test_adapter_falls_back_to_environ_without_scope(self):
|
||||
"""Without a secret scope (single-profile mode), the adapter reads
|
||||
from os.environ — backward-compatible with legacy behavior."""
|
||||
from gateway.config import PlatformConfig
|
||||
from plugins.platforms.email.adapter import EmailAdapter
|
||||
|
||||
cfg = PlatformConfig(enabled=True)
|
||||
adapter = EmailAdapter(cfg)
|
||||
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")
|
||||
|
||||
@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",
|
||||
}, clear=False)
|
||||
def test_check_email_requirements_uses_scope(self):
|
||||
"""check_email_requirements must also honor the secret scope so that
|
||||
a secondary profile with scoped email creds is detected as configured."""
|
||||
scoped = {
|
||||
"EMAIL_ADDRESS": "beta@test.invalid",
|
||||
"EMAIL_PASSWORD": "secondary-pw",
|
||||
"EMAIL_IMAP_HOST": "imap.secondary.example",
|
||||
"EMAIL_SMTP_HOST": "smtp.secondary.example",
|
||||
}
|
||||
ss.set_multiplex_active(True)
|
||||
token = ss.set_secret_scope(scoped)
|
||||
try:
|
||||
from plugins.platforms.email.adapter import check_email_requirements
|
||||
self.assertTrue(check_email_requirements())
|
||||
finally:
|
||||
ss.reset_secret_scope(token)
|
||||
|
||||
@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",
|
||||
}, clear=False)
|
||||
def test_adapter_scoped_missing_key_does_not_leak_environ(self):
|
||||
"""If a key is absent from the scope but present in os.environ,
|
||||
the adapter must NOT fall through to os.environ (which would leak
|
||||
the default profile's value)."""
|
||||
from gateway.config import PlatformConfig
|
||||
from plugins.platforms.email.adapter import EmailAdapter
|
||||
|
||||
scoped = {
|
||||
"EMAIL_ADDRESS": "beta@test.invalid",
|
||||
# EMAIL_PASSWORD intentionally missing from scope
|
||||
"EMAIL_IMAP_HOST": "imap.secondary.example",
|
||||
"EMAIL_SMTP_HOST": "smtp.secondary.example",
|
||||
}
|
||||
ss.set_multiplex_active(True)
|
||||
token = ss.set_secret_scope(scoped)
|
||||
try:
|
||||
cfg = PlatformConfig(enabled=True)
|
||||
adapter = EmailAdapter(cfg)
|
||||
self.assertEqual(adapter._address, "beta@test.invalid")
|
||||
# Password must NOT be the default profile's "default-pw"
|
||||
self.assertNotEqual(adapter._password, "default-pw")
|
||||
self.assertEqual(adapter._password, "")
|
||||
finally:
|
||||
ss.reset_secret_scope(token)
|
||||
|
||||
@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_ALLOWED_USERS": "gamma@test.invalid",
|
||||
}, clear=False)
|
||||
def test_allowed_users_uses_scope(self):
|
||||
"""EMAIL_ALLOWED_USERS must also be read from the secret scope
|
||||
so a secondary profile gets its own allowlist, not the default's."""
|
||||
scoped = {
|
||||
"EMAIL_ADDRESS": "beta@test.invalid",
|
||||
"EMAIL_PASSWORD": "secondary-pw",
|
||||
"EMAIL_IMAP_HOST": "imap.secondary.example",
|
||||
"EMAIL_SMTP_HOST": "smtp.secondary.example",
|
||||
"EMAIL_ALLOWED_USERS": "epsilon@test.invalid,delta@test.invalid",
|
||||
}
|
||||
ss.set_multiplex_active(True)
|
||||
token = ss.set_secret_scope(scoped)
|
||||
try:
|
||||
# _allowlist_in_effect reads EMAIL_ALLOWED_USERS — verify it
|
||||
# sees the scoped value, not the environ value
|
||||
from plugins.platforms.email.adapter import EmailAdapter
|
||||
self.assertTrue(EmailAdapter._allowlist_in_effect())
|
||||
finally:
|
||||
ss.reset_secret_scope(token)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Loading…
Reference in New Issue