Skip to content

fix: guard float() env var casts against ValueError (16 locations) - #32458

Open
annguyenNous wants to merge 1 commit into
NousResearch:mainfrom
annguyenNous:fix/float-env-var-guards
Open

fix: guard float() env var casts against ValueError (16 locations)#32458
annguyenNous wants to merge 1 commit into
NousResearch:mainfrom
annguyenNous:fix/float-env-var-guards

Conversation

@annguyenNous

Copy link
Copy Markdown
Contributor

Summary

Guards 16 unguarded float(os.getenv(...)) calls across 8 files against ValueError when env vars contain non-numeric strings.

Bug Pattern

# BEFORE (bug): crashes with ValueError if env var is "abc"
timeout = float(os.getenv("HERMES_API_TIMEOUT", "1800"))

# AFTER (safe): falls back to default on invalid input
timeout = _safe_float_env("HERMES_API_TIMEOUT", 1800.0)

The or default pattern (float(os.getenv("X") or 15)) does NOT protect against this — "abc" is truthy, so or never fires.

Files Fixed

File Locations Method
agent/chat_completion_helpers.py 3 _safe_float_env helper
agent/auxiliary_client.py 2 _safe_float_env helper
run_agent.py 2 _safe_float_env helper
hermes_cli/auth.py 2 _safe_float_env helper
hermes_cli/runtime_provider.py 2 _safe_float_env helper
gateway/platforms/telegram.py 1 inline try/except
gateway/platforms/wecom.py 2 inline try/except
plugins/platforms/discord/adapter.py 2 inline try/except

Impact

A typo in any of these env vars (e.g., HERMES_API_TIMEOUT=abc) currently crashes the entire agent/gateway on startup. With this fix, it logs a warning and uses the default value.

Test Plan

  • All 8 modified files pass py_compile
  • Set HERMES_API_TIMEOUT=abc and verify agent starts with default timeout
  • Set HERMES_DISCORD_TEXT_BATCH_DELAY_SECONDS=xyz and verify Discord adapter starts

float(os.getenv("VAR", "default")) crashes with ValueError when the
env var contains a non-numeric string like "abc". The `or default`
pattern only protects against falsy values (None, ""), not wrong-type
truthy values.

This fixes 16 unguarded float() casts across 8 files:

Core agent (with _safe_float_env helper):
- agent/chat_completion_helpers.py: HERMES_API_TIMEOUT,
  HERMES_STREAM_READ_TIMEOUT, HERMES_STREAM_STALE_TIMEOUT
- agent/auxiliary_client.py: HERMES_NOUS_TIMEOUT_SECONDS (2 locations)
- run_agent.py: HERMES_API_TIMEOUT, HERMES_NOUS_TIMEOUT_SECONDS

CLI:
- hermes_cli/auth.py: HERMES_CODEX_REFRESH_TIMEOUT_SECONDS,
  HERMES_XAI_REFRESH_TIMEOUT_SECONDS
- hermes_cli/runtime_provider.py: HERMES_NOUS_TIMEOUT_SECONDS (2 locations)

Platform adapters (inline try/except):
- gateway/platforms/telegram.py: HERMES_TELEGRAM_MEDIA_BATCH_DELAY_SECONDS
- gateway/platforms/wecom.py: HERMES_WECOM_TEXT_BATCH_DELAY_SECONDS,
  HERMES_WECOM_TEXT_BATCH_SPLIT_DELAY_SECONDS
- plugins/platforms/discord/adapter.py: HERMES_DISCORD_TEXT_BATCH_DELAY_SECONDS,
  HERMES_DISCORD_TEXT_BATCH_SPLIT_DELAY_SECONDS

Files with 3+ locations use a _safe_float_env() helper; files with 1-2
locations use inline try/except for minimal diff.
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for identifying a real malformed-environment failure. Most of this PR's original sites were independently migrated to the canonical helper in a7dd98c860; two runtime-provider paths still need a focused salvage.

Problems

  • hermes_cli/runtime_provider.py:1441 and :1765 still call float(_getenv("HERMES_NOUS_TIMEOUT_SECONDS", "15")), so a non-numeric scoped value raises ValueError.
  • The proposed _safe_float_env reads os.getenv, but current runtime_provider._getenv at hermes_cli/runtime_provider.py:49-59 is profile-scoped through get_secret. A salvage must retain that lookup path.
  • run_agent.py:1257-1259 is a current sibling unguarded float conversion that should be included or tracked separately.

Suggested changes

  • Retain only the two remaining runtime-provider fixes, parse the _getenv result safely, and add regression tests for both Nous resolution branches.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants