feat(health): support non-HTTP health checks for TCP/CLI services (#666)#1343
feat(health): support non-HTTP health checks for TCP/CLI services (#666)#1343Arifuzzamanjoy wants to merge 6 commits into
Conversation
|
Thanks for the PR. I think this is a genuinely useful feature: the current HTTP-only model gives false health signals for services like Piper/Wyoming and CLI/no-server tools like Aider, and I verified the main contract tests locally against a branch merged with current
CI is also green. The one thing I want fixed before merge is the TCP probe shell invocation:
bash -c "cat < /dev/null > /dev/tcp/127.0.0.1/$port"
Since the port can come from manifest/env-derived configuration, please treat it as untrusted input. A malformed value should not be able to become shell text. A simple guard like Once that TCP probe hardening is in, I think this is a valuable and mergeable change. |
Apply the same input-validation + positional-arg pattern from 1f47797 to the remaining unguarded bash -c /dev/tcp interpolation in test_tcp(). Validates host against [a-zA-Z0-9.:-] and port against numeric 1-65535 before shell expansion.
2ecb039 to
54d5d50
Compare
|
Two commits added for the TCP probe hardening:
No unguarded |
Closes #666. Flagged the approach in the issue and didn't hear back — opening this for review, happy to redirect if the direction's wrong.
Adds
health_type: http | tcp | noneto the service manifest. Defaults tohttp, so existing manifests don't need changes.http— unchangedtcp— opens a connection to the port, short timeout, no HTTP path needednone— skip the check, show as N/AWired through schema, audit,
service-registry.sh,health-check.sh,dream-doctor.sh, and dashboard-api so all five surfaces agree on the same states.Real services updated
piper-audio→tcp(Wyoming protocol, not HTTP)aider→none(CLI tool, no server)Tests
Health-check test spins up a real HTTP server and TCP listener and verifies
ok / ok / not_applicable. Audit test also rejects an invalidhealth_type: smtpfixture.21 files, +553 / -182. No new dependencies. Happy to split schema + plumbing from the manifest updates if you'd prefer.