From f08f403157d36d2c31b6329ca79740fe085f7158 Mon Sep 17 00:00:00 2001 From: shikanga-hermes Date: Sun, 5 Jul 2026 19:22:13 +0000 Subject: [PATCH] fix(email): honor profile secret scope for email adapter env reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- plugins/platforms/email/adapter.py | 37 +++--- tests/gateway/test_email_secret_scope.py | 161 +++++++++++++++++++++++ 2 files changed, 181 insertions(+), 17 deletions(-) create mode 100644 tests/gateway/test_email_secret_scope.py diff --git a/plugins/platforms/email/adapter.py b/plugins/platforms/email/adapter.py index 572b5c11455d3..3bc61cf787b3d 100644 --- a/plugins/platforms/email/adapter.py +++ b/plugins/platforms/email/adapter.py @@ -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 diff --git a/tests/gateway/test_email_secret_scope.py b/tests/gateway/test_email_secret_scope.py new file mode 100644 index 0000000000000..693f750f39007 --- /dev/null +++ b/tests/gateway/test_email_secret_scope.py @@ -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()