Skip to content

fix(config): guard WECOM/BlueBubbles port env vars against malformed values - #49416

Closed
vanthinh6886 wants to merge 1 commit into
NousResearch:mainfrom
vanthinh6886:fix/guard-port-env-vars-in-config
Closed

fix(config): guard WECOM/BlueBubbles port env vars against malformed values#49416
vanthinh6886 wants to merge 1 commit into
NousResearch:mainfrom
vanthinh6886:fix/guard-port-env-vars-in-config

Conversation

@vanthinh6886

Copy link
Copy Markdown
Contributor

Summary

WECOM_CALLBACK_PORT and BLUEBUBBLES_WEBHOOK_PORT in gateway/config.py use bare int(os.getenv(...)) which raises ValueError if the env var contains a non-numeric string (e.g. "", "abc"). This crashes the gateway on startup.

Fix

Replace with env_int() from utils.py which gracefully falls back to the default value on malformed input. This is the same pattern already used throughout the rest of the codebase.

Changes

  • gateway/config.py: import env_int from utils, replace 2 bare int(os.getenv(...)) calls

Test Plan

  • Gateway config tests pass (67 passed)

@xg-gh-25

Copy link
Copy Markdown

Nice defensive coding. Guarding port env vars against malformed values prevents a class of runtime crashes that are surprisingly common in containerized/k8s deployments where env vars get templated from ConfigMaps.

What this PR likely prevents:

  • int("") crashes when the var is set but empty (common typo: WECOM_PORT= instead of unset)
  • Invalid port ranges (e.g., someone pastes "8080-8090" thinking it's a range)
  • Negative/zero ports (accidental -8080 or 0)

Best practice pattern:

def parse_port(env_var: str, default: int = 8080) -> int:
    val = os.getenv(env_var, "").strip()
    if not val:
        return default
    try:
        port = int(val)
        if not (1 <= port <= 65535):
            raise ValueError(f"Port out of range: {port}")
        return port
    except ValueError as e:
        logger.warning(f"Invalid {env_var}={val!r}, using default {default}: {e}")
        return default

If WECOM and BlueBubbles are notification channels, crashing on startup (before the fix) would block all notifications. Good catch.

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Jun 20, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #48748 — identical diff (same author): both replace the two bare int(os.getenv(...)) casts for WECOM_CALLBACK_PORT and BLUEBUBBLES_WEBHOOK_PORT in gateway/config.py with env_int(...). #48748 is the earlier still-open PR and is the canonical version. Related: #48368 (broader bare-int/float env guard across platform configs).

…values

WECOM_CALLBACK_PORT and BLUEBUBBLES_WEBHOOK_PORT use bare int(os.getenv(...))
which raises ValueError if the env var contains a non-numeric string (e.g.
"abc" or ""). Replace with env_int() from utils which gracefully falls back
to the default value on malformed input.
@vanthinh6886
vanthinh6886 force-pushed the fix/guard-port-env-vars-in-config branch from 37e7cc6 to 7810e1c Compare June 20, 2026 06:50
@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Closing as superseded by #49558, which landed the canonical fix for this whole bug class.

#49558 adds env_float() alongside the existing env_int() in utils.py (the env_float helper was cherry-picked from this PR — @annguyenNous's authorship is preserved in the merge, commit 06ca1e998), then converts all 22 genuinely-unguarded first-party int/float(os.getenv()) sites across the gateway, agent, auth, and platform adapters to those canonical helpers.

We went with the utils.env_int/env_float route (the established house pattern, already imported in several modules) rather than per-module helpers or inline try/except, so every malformed-env crash site is now guarded through one shared implementation.

Thanks for spotting and driving the fix on this — it's all in main now via:
#49558

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants