fix(deepseek): explicit RuntimeError on missing env (Squad G / E2E Blocker Elimination) - #145
Conversation
Discovery audit Agent #3 (2026-05-09) flagged this as a real security risk during the E2E Blocker Elimination Program audit pass. Pre-fix - build_probe_request and completion_caller used os.environ[config.api_key_env] which raises a bare KeyError when the env var is missing. - Bare KeyError surfaced as opaque HTTP 500 with the env variable NAME in the traceback — low-risk info disclosure (the env-var NAME, not the value, but still preferable to be explicit). Post-fix - Both functions use os.environ.get(config.api_key_env) and explicitly raise RuntimeError("DeepSeek provider is not configured: ...") with an actionable operator-facing message that does NOT name the env var. - Behavior is otherwise unchanged on the happy path. Tests - test_build_probe_request_missing_env_raises updated to expect RuntimeError with match string. - New test_completion_caller_missing_env_raises pins the same guard for the completion path. Verification - py_compile: OK - Focused tests: 6/6 pass on test_deepseek.py - Pre-push hook: passed Scope - DeepSeek only. MiniMax already had explicit configured-flag handling (Agent 14 prior swarm verified). No other providers touched. Discovery provenance - Discovery audit Agent #3 of the E2E Blocker Elimination Program found this as one of 2 security risks (the other being broad-except cleanup in 6 sites; tracked as separate follow-up). References - OWASP A01:2021 Broken Access Control / A09 Logging https://owasp.org/Top10/A09_2021-Security_Logging_and_Monitoring_Failures/ Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request replaces implicit KeyError exceptions with explicit RuntimeError messages when the DeepSeek API key is missing from the environment, preventing potential information disclosure of environment variable names in tracebacks. These changes were applied to both the probe request construction and the completion caller. Feedback was provided to move the configuration check in completion_caller from call-time to initialization-time to implement a 'fail-fast' pattern, along with corresponding test updates.
| caller = deepseek.completion_caller(_config()) | ||
| request = LLMRequest(prompt="hello", max_completion_tokens=10, token_id="test-token-id") | ||
| with pytest.raises(RuntimeError, match="DeepSeek provider is not configured"): | ||
| caller(request) |
There was a problem hiding this comment.
This test should be updated to reflect the move of the configuration check from call-time to initialization-time in completion_caller. This ensures that the 'fail-fast' behavior is correctly verified.
| caller = deepseek.completion_caller(_config()) | |
| request = LLMRequest(prompt="hello", max_completion_tokens=10, token_id="test-token-id") | |
| with pytest.raises(RuntimeError, match="DeepSeek provider is not configured"): | |
| caller(request) | |
| with pytest.raises(RuntimeError, match="DeepSeek provider is not configured"): | |
| deepseek.completion_caller(_config()) |
…5-09) (#146) User-mandated synthesis after all 10 wave agents returned with research receipts. Gates code-PR resumption per execution order. Sections (8, per user mandate) 1. What remains blocked (BLK-009 server-contract, BLK-011 upstream, BLK-016 proof) 2. What was skipped/deferred (Bonus 12 #9, #10, MiniMax parity, 10 broad-except, BLK-018, RC v2 commits 2-5, BLK-013, BLK-014, BLK-015, BLK-019, BLK-020) 3. What can be fixed now (10 PR-buildable items in ascending risk order) 4. What needs upstream / env / user action 5. Next 5 PRs in exact order: PR #146 #147 #148 #149 #150 6. Hermes Agent v0.13 retry: NO - KEEP DEFERRED (Joint Agent 1+3 verdict) 7. RC v2 resume: YES (all preconditions met; commit 2 ready) 8. OpenCode/OpenHands real-task proof: YES with sandbox hardening Decision points - v0.13 retry: CLOSED (Agent 1+3 both NO) - RC v2 resume: OPEN (Agent 2 + Agent 4 both YES; needs user authorization) - GUI Playwright: OPEN dashboard-advanced ONLY (Agent 9 says ready) - Code PR freeze: lifts after this synthesis lands Critical findings banked - Agent 1: PR #22567 (Windows pwd/fcntl skip-guards) closed-not-merged. Real upstream red is product regressions (gateway.draining translation-key, TTS routing async-mock), NOT Windows guards. - Agent 2: NO Hermes3D feature broken by v0.12; v0.13 lift is forward-investment not blocker-clearance. - Agent 3: Lane 1 Windows host CANNOT certify v0.13 by construction. - Agent 6: Bonus 12 #9 + #10 still open + READY-TO-PR (mechanical). - Agent 7: MiniMax has same KeyError pattern PR #145 fixed for DeepSeek; 10 broad-except cleanup sites enumerated. - Agent 8: 60-app first 5-row backfill ready (prusaslicer, orcaslicer, blender, trimesh, manifold). - Agent 9: dashboard-advanced is FIRST visual target genuinely ready; Squad E was wrong about settings-root testid. - Agent 10: BLK-016 is PROOF blocker not code blocker; 4 hard gates unit-tested; drill plan ready. All 10 agents produced 2+ research receipts (1 primary + 1 cross-comparison). Two agents reported "no new evidence vs prior swarm" honestly and stopped per the 2-loop escalation rule. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Wave synthesis 2026-05-09) (#148) Mirrors PR #145 DeepSeek fix. Wave Agent 7 of the Remaining/Skipped Blocker Wave found the same bare-KeyError pattern in MiniMax that PR #145 fixed for DeepSeek. Pre-fix - _minimax_api_key raised bare KeyError(config.api_key_env) when no env candidate matched. The KeyError surface in tracebacks named the env variable, low-risk info disclosure. Post-fix - Explicit RuntimeError("MiniMax provider is not configured: API key env variable is unset. Set the configured key in the private env file before invoking the provider.") with NO env variable name echoed. - Behavior otherwise unchanged on the happy path. Tests - test_build_probe_request_missing_env_raises updated to expect RuntimeError with match string (parity with PR #145 DeepSeek test). Verification - py_compile: OK - Focused tests: 6/6 pass on test_minimax.py - Pre-push hook: passed Scope - MiniMax parity ONLY. Broad-except cleanup at 7 confirmed sites (Wave Agent 7) deferred to a follow-up PR for review hygiene (touches 7 different files; needs logger setup in 4 of them). Swarm provenance - Wave Agent 7 of the 10-agent Remaining/Skipped Blocker Wave produced the parity diff; orchestrator implemented + tested. References - PR #145 (b5b925a) DeepSeek same fix - OWASP A09 Logging & Monitoring Failures Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…AIL (#157) Canary smoke run executed against G:/Github/hermes-agent-v013-canary (v2026.5.7, sha 498bfc7) with HERMES_AGENT_CHECKOUT env switch from PR #155. v2026.5.9 does NOT exist upstream; latest tag remains v2026.5.7 (per user rule: "Use candidate v2026.5.9 if available, otherwise latest post-v2026.5.7 cleanup candidate"). Results 1. Hermes Agent imports — PASS (8/8 top-level packages) 2. MCP tools load — PASS (10 @mcp.tool() decorators in mcp_serve.py) 3. MiniMax — PASS (config layer, auth header redacted, no key printed) 4. DeepSeek — PASS (graceful RuntimeError when env unset; PR #145/#148 fix active) 5. OpenCode preflight — PASS (detected v1.4.3-hermes3d) 6. OpenHands preflight — PASS (detected CLI 1.16.0) 7. Bounded coding/audit task — N/A (BLK-013 endpoint not yet shipped; honest deferral, not skip) 8. Rollback to v0.12 — PASS (mid-process flip-and-back works canary->prod->canary->prod without process restart) Production safety - G:/Github/hermes-agent-fresh HEAD unchanged (73bf3ab1b223, v2026.4.30) - git status --short empty post-smoke - Production checkout byte-identical to pre-smoke state Promotion proposal - CONDITIONAL: 7 PASS / 1 N/A / 0 FAIL clears safety bar - Three operator-driven gates before flipping the default: 1. Live MiniMax + DeepSeek probes (deferred in S3/S4 to avoid credit spend; integration path verified, actual probe is operator step) 2. BLK-013 bounded-task PR ships OR operator approves Smoke 7 defer 3. Upstream tui_gateway/entry.py Windows guard merges (only required for TUI dashboard / PTY chat surface) - If accepted: 1-line change in services/agent_checkout.py + ~5 LoC tests; easy rollback via revert - If declined: status quo (production v0.12 default, canary opt-in via env) holds with zero code change Constraints honored - Production v0.12 untouched - Canary opt-in only - No secrets printed - No broad skip (Smoke 7 has documented blocker) - Wired via env/config only - Rollback proven Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…85) (#198) Adds H3D-CLOSED-PR-LEDGER.md as the source-of-truth for the 19 closed-unmerged PRs in the Wave 10 audit scope. Each row cites the merged successor PR(s) and proof file paths, satisfying the feedback_weakness_correction.md rule that every PARTIAL audit finding must be paired with a fix-PR. The PR-#85 row in particular records the fold-in chain attribution (#84 team assignment -> #104 provider smoke -> #107/#112/#124/#145/#148 hardening) that was missing from the replacement-PR bodies per W10-A9's anti-rubber-stamp finding. Also includes a See also link from the W10 audit synthesis (CLOSED_UNMERGED_PR_SUPERSESSION_AUDIT_2026-05-10.md) to the new ledger, and a PARTIAL gap reconciliation section enumerating the Wave 11 closing PRs for #37 (W11-2 UI Playwright spec), #83 (W11-3 5 git-contract unit tests), and #85 (this PR — ledger doc). Doc-only PR: no source/test edits. Hermes evidence chain: PASS Task ID: W11-4-PR85-LEDGER-2026-05-10 Hermes lock owner: claude-w11-4-pr85 hermes_run_gate: docs-only-truth-gate Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Discovery audit Agent #3 (2026-05-09) flagged this during the E2E Blocker Elimination Program audit pass.
build_probe_requestandcompletion_callerpreviously usedos.environ[config.api_key_env]which raises a bareKeyErrorwhen the env var is missing. The bare KeyError surfaced as an opaque HTTP 500 with the env variable NAME in the traceback (low-risk info disclosure).Fix
Both functions now use
os.environ.get(...)and explicitly raiseRuntimeError("DeepSeek provider is not configured: ...")with an actionable operator-facing message that does NOT name the env var.Tests (6/6 pass)
test_build_probe_request_missing_env_raisesupdated to expect RuntimeError with match stringtest_completion_caller_missing_env_raisespins the same guard for the completion pathTest plan
Scope
Discovery provenance
Discovery audit Agent #3 of the E2E Blocker Elimination Program. Tracks as one of 2 security risks (the other = broad-except cleanup at 6 sites; separate PR follow-up).
🤖 Generated with Claude Code