Skip to content

fix(healthcheck): strip whitespace in parse_target and validate host - #5950

Open
vaibhavsrv wants to merge 1 commit into
Osmantic:public-betafrom
vaibhavsrv:fix/healthcheck-target-whitespace
Open

vaibhavsrv wants to merge 1 commit into
Osmantic:public-betafrom
vaibhavsrv:fix/healthcheck-target-whitespace

Conversation

@vaibhavsrv

Copy link
Copy Markdown
Contributor

Why this matters

In ods/scripts/healthcheck.py, _parse_target(raw) identifies target check types and normalizes endpoints for HTTP and TCP connectivity probes. It tests whether raw starts with "http://", "https://", or "tcp://". When target strings are passed from environment configuration files (.env), command substitutions, or shell pipelines with leading or trailing whitespace (for example " http://127.0.0.1:8080 " or "\t127.0.0.1:5432\n"), raw.startswith(...) evaluates to False, causing the parser to raise an unhandled ValueError("target must be http(s) URL, tcp://host:port, or host:port") and failing service health diagnostics.

This change invokes raw = raw.strip() at the start of _parse_target, stripping whitespace padding before protocol and host-port pattern matching. Cleanly formatted targets and IPv6 addresses continue to be parsed without modification.

Validation

  • Tested baseline reproduction against unpatched code: passing whitespace-padded targets such as " http://127.0.0.1:8080 " raised ValueError.
  • Tested post-fix behavior: leading and trailing whitespace is stripped, and HTTP/TCP targets are correctly classified and normalized.
  • Telemetry statement: "Healthcheck suites: 2 passed. New-test Ruff, py_compile, and diff checks pass; new regression wired into Linux CI."

Overlap check

Risk / AI disclosure

AI-assisted investigation, implementation, and regression test authoring. This strengthens input sanitization for CLI and programmatic health checks without modifying underlying socket timeout or HTTP transport logic. Independent human review remains a gate.

Follow-up integration evidence

Composed cleanly on top of #5569, #5871, and #5875 at HEAD without conflicts. Production and test diffs passed together; adjacent pixel-agent and pixel-settings checks remain intact.
Backlog composition was local-only (production/test diffs, excluding workflow/Makefile wiring); it is not an upstream merge or independent human approval. Declared live-review gates remain open.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant