fix(email): honor profile secret scope for email adapter env reads - #59076
phantom-instruction-set wants to merge 1 commit into
Conversation
8dbb12d to
b159fe6
Compare
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 (NousResearch#52307/NousResearch#50051): the same os.getenv→get_secret migration that PR NousResearch#50094 applies to gateway/config.py, but for the email adapter itself, which neither PR NousResearch#50094 nor NousResearch#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 NousResearch#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: NousResearch#50051, NousResearch#52307, PR NousResearch#50094, PR NousResearch#51374
b159fe6 to
18b70b4
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the adapter-side gap. The premise is still valid on current main: plugins/platforms/email/adapter.py:437-443 reads email credentials from os.getenv() even though multiplexed adapter creation runs under _profile_runtime_scope() (gateway/run.py:8657-8689), which installs a profile secret scope (gateway/run.py:1441-1472).
Problems
- The
gateway/config.pyportion is already superseded by0f154e780e71c74f8a1cdccb25c97a6abd8e5a57: currentgateway/config.py:174-204provides_getenv, andgateway/config.py:1717-1736already routes the Email block through it. - The adapter retains direct reads for
EMAIL_IMAP_PORT,EMAIL_SMTP_PORT, andEMAIL_POLL_INTERVALatplugins/platforms/email/adapter.py:440-443, plusEMAIL_TRUST_FROM_HEADERat line 467. - The new allowlist test only checks a truthy result; its default-profile and scoped values are both nonempty, so it would pass before the migration.
Suggested changes
- Salvage the adapter/test portion only, complete the remaining per-profile
EMAIL_*reads, and make the allowlist regression test distinguish scoped from process-global input.
Automated hermes-sweeper review.
| 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", "") |
There was a problem hiding this comment.
Please complete the scope migration for the adjacent per-profile Email settings too: EMAIL_IMAP_PORT, EMAIL_SMTP_PORT, and EMAIL_POLL_INTERVAL still use env_int, and EMAIL_TRUST_FROM_HEADER still uses env_bool. In multiplex mode those helpers read process-global environment values rather than the profile scope.
… + 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 5438e9c (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.
… + scope ports/trust flag Follow-up to the salvaged NousResearch#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 43e0fe7 (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.
… + scope ports/trust flag Follow-up to the salvaged NousResearch#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 754cdd2 (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.
… + scope ports/trust flag Follow-up to the salvaged NousResearch#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 5438e9c (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.
What does this PR do?
The email adapter (
plugins/platforms/email/adapter.py) readEMAIL_ADDRESS,EMAIL_PASSWORD,EMAIL_IMAP_HOST,EMAIL_SMTP_HOST,EMAIL_ALLOWED_USERS, andEMAIL_ALLOW_ALL_USERSviaos.getenv()directly. In a multiplexed gateway,os.environholds the default profile's.envvalues, so every secondary profile inherited the default profile's email credentials instead of its own — the adapter polled the wrong inbox and used the wrong allowlist.This is a sibling of the
api_serverenv-leak bug (#52307/#50051): the sameos.getenv→get_secretmigration that PR #50094 applies togateway/config.py, but for the email adapter itself, which neither PR #50094 nor #51374 covers.Related Issue
Sibling of #50051 and #52307. Covers the email adapter gap left by PR #50094 (which fixes
gateway/config.pyonly).Type of Change
Changes Made
plugins/platforms/email/adapter.py: replaceos.getenvwithagent.secret_scope.get_secretfor allEMAIL_*credential reads (adapter__init__,check_email_requirements,_allowlist_in_effect,_dispatch_messageallowlist gate,_send_emailSMTP helper,EMAIL_AUTHSERV_ID).gateway/config.py: add_getenv/_getenv_str/_getenv_inthelpers (from PR fix(gateway): isolate multiplex profile config env reads #50094) and replaceos.getenvwith_getenvfor the email block in_apply_env_overrides.tests/gateway/test_email_secret_scope.py: 5 new tests.How to Test
Expected: 5 passed.
Verify the RED→GREEN cycle: stash the adapter.py change, re-run — 2 tests FAIL. Restore — all 5 pass.
Run existing suites (no regressions):
Expected: 219 passed.
Checklist
Code
fix(email):)gateway/config.pybut neither touchesplugins/platforms/email/adapter.pyDocumentation & Housekeeping