fix: fail closed for account-scoped publish credentials

[study8677]

Motivation

• Close an authorization gap where account-scoped publish credentials (Reddit/Twitter) could be resolved from admin/system rows or the process environment, allowing a non-admin tenant to publish using another account's credentials.
• Ensure publish-provider secrets are strictly isolated to the request or the owning account to prevent cross-tenant credential reuse.

Description

• Add _ACCOUNT_SCOPED_SECRET_KEYS in src/opencmo/llm.py listing publish-related secrets (REDDIT_, TWITTER_).
• Harden llm.get_key and llm.get_key_async so that if a requested key is in _ACCOUNT_SCOPED_SECRET_KEYS and it is not present in the request ContextVar, the account snapshot, or the account DB row, the resolver returns default (fails closed) instead of falling through to system/admin DB or os.environ.
• Add a regression test test_publish_credentials_do_not_fallback_to_env_or_system in tests/test_settings_multitenant.py that asserts a tenant without Reddit credentials cannot obtain them from admin account settings or environment.

Testing

• Ran pytest -q tests/test_llm.py tests/test_settings_multitenant.py (without PYTHONPATH) which failed to collect due to import path issues in this environment (ModuleNotFoundError for opencmo).
• Ran PYTHONPATH=src pytest -q tests/test_llm.py tests/test_settings_multitenant.py, which executed but many async-marked tests failed because the test environment is missing async test plugins/dependencies (e.g. pytest-asyncio), so the full suite could not be validated here.
• Ran a focused invocation for the new/related multitenant tests (PYTHONPATH=src pytest -q tests/test_settings_multitenant.py -k "publish_credentials_do_not_fallback_to_env_or_system or get_key_reads_account_snapshot_set_by_middleware"), but execution was skipped in this environment due to test prerequisites (FastAPI/other runtime test deps) not being available; the new regression test is present and will run in CI where the project test deps are installed.

---

Codex Task

[study8677]

Code Review — PR #21

总体评价

安全意图正确——关闭跨账号凭据泄漏漏洞的方向无误，回归测试覆盖了负向路径。但当前实现对 account_id is None 的上下文（CLI、后台任务、单租户本地部署）过度限制，会造成静默破坏性变更，且两个函数的 docstring 均未更新。建议在修复范围问题后再合并。

---

问题清单

级别	文件 & 行号	描述	建议
🟡 建议	llm.py:240-242	过度限制——无账号上下文时也 fail-closed。当 account_id is None（CLI opencmo 命令、APScheduler 后台任务、单租户 .env 配置场景），步骤 1-3 全部未命中后，步骤 4 直接返回 None，导致 os.environ/.env 里的 REDDIT_CLIENT_ID 等凭据完全失效。跨账号凭据泄漏只在 有账号绑定且该账号缺少凭据 时才会发生；无账号上下文属于合法的单租户使用，应允许继续走 env 回退。	将检查改为 if name in _ACCOUNT_SCOPED_SECRET_KEYS and account_id is not None: return default
🟡 建议	llm.py:193-194	同上——get_key() 同步版本同样过度限制。同步函数没有 account_id 引用，判断条件里应先取 _current_account_id.get(None) 再加 and account_id is not None。	见修改示例
🟡 建议	llm.py:168-177	get_key() docstring 未更新。注释仍写 "3. os.environ (from .env or system)"，但实际步骤 3 已变为 fail-closed 检查，os.environ 降为步骤 4，文档与实现不一致。	更新 Resolution order，增加步骤 3 的描述
🟡 建议	llm.py:204-215	get_key_async() docstring 未更新。Docstring 仍列出旧的步骤顺序，完全未提及新增的 account-scoped fail-closed 逻辑（步骤 4）。	正确列出含新步骤 4 的完整顺序
🟢 优化	llm.py:63-72	_ACCOUNT_SCOPED_SECRET_KEYS 定义处缺少威胁模型说明，新增这个 frozenset 的目的对后续维护者不直观。	在 frozenset 上方加注释：# Publish-platform secrets; must not leak across tenants via env/system fallback.
🟢 优化	tests/test_settings_multitenant.py:179-193	测试仅覆盖负向路径（租户无凭据 → 返回 None），缺少 account_id is None 时的行为测试——修复后应允许 env 回退，但目前没有测试锁定这个边界。	补充 test_publish_credentials_resolve_from_env_when_no_account_bound

---

亮点

• 使用 frozenset 定义受保护 key 集合，查找 O(1)，扩展方便。
• get_key_async 的新检查（步骤 4）位置精准：在完成账号维度的所有查找（ContextVar → snapshot → DB）之后、进入系统级回退之前插入，逻辑链清晰。
• 回归测试结构完整：try/finally 确保 ContextVar token 必被重置，不影响其他测试；同时验证了 async 和 sync 两条路径。
• _ACCOUNT_SCOPED_SECRET_KEYS 独立于 _ENV_PRIORITY_KEYS，两者不相交，无优先级冲突。

---

修改示例

核心修复：将 fail-closed 限定为"有账号绑定"时才生效

get_key_async（llm.py:240-242）：

# 4. Sensitive account-scoped secrets must not fall through to system/env
#    when a tenant account is bound (prevents cross-tenant credential bleed).
#    When no account is bound (CLI, background tasks, single-tenant env),
#    allow the normal env/system fallback to proceed.
if name in _ACCOUNT_SCOPED_SECRET_KEYS and account_id is not None:
    return default

get_key（llm.py:191-194）：

# 3. Fail closed only when a tenant account context is active.
account_id = _current_account_id.get(None)
if name in _ACCOUNT_SCOPED_SECRET_KEYS and account_id is not None:
    return default

补充边界测试

def test_publish_credentials_resolve_from_env_when_no_account_bound(monkeypatch):
    """Without an account context (CLI / background), env vars must still work."""
    monkeypatch.setenv("REDDIT_CLIENT_ID", "env-cid")
    # No account bound — ContextVar defaults to None
    assert llm.get_key("REDDIT_CLIENT_ID") == "env-cid"
    assert asyncio.run(llm.get_key_async("REDDIT_CLIENT_ID")) == "env-cid"

更新 get_key_async docstring

"""Get a configuration key with multi-tenant DB fallback (async version).

Resolution order:
    1. ContextVar (per-request BYOK keys)
    2. Per-account settings snapshot bound by middleware
    3. ``account_settings`` row keyed off the current account_id ContextVar
    4. Fail closed for account-scoped publish secrets **when an account is
       bound** (prevents cross-tenant credential bleed)
    5. os.environ (router-default keys checked early via ``_ENV_PRIORITY_KEYS``)
    6. System-level fallback (admin account row → legacy ``settings`` table)
    7. os.environ (final fallback)
    8. ``default``
"""

---

Generated by Claude Code

[study8677]

Incorporated via #27 → main.