fix(inputs): fail a flow or dnstap listener that cannot bind instead of running without it by leoparente · Pull Request #805 · netboxlabs/pktvisor

leoparente · 2026-10-29T00:58:41Z

Summary

Fixes MON-713. QA found that a second pktvisor flow policy on a UDP port already in use, with a different input.config, was accepted and reported running. pktvisord answered 201 and ran two input streams, but only one socket listened on the port. The docs say pktvisor refuses that policy.

Cause. uvw 3 returns libuv's result from bind(), listen() and recv() rather than emitting an error event. The flow input and both dnstap servers ignored those results. So a bind that failed with "address already in use" went unnoticed, start() succeeded, and the input ran on a socket that never got the port.

Fix.

Listeners that can't bind fail the start: FlowInputStream and DnstapInputStream (TCP and Unix) check each call. On failure they close the handles and the loop they created, then throw with libuv's reason. The policy load then fails and is rolled back (fix(policies): roll back every failed load and detach it from shared inputs #803), and orb-agent shows the policy failed_to_apply with, for example, unable to bind flow UDP server on 0.0.0.0:6343: address already in use.
IPv6 flow binds work. uvw's bind(ip, port) passes the address through a helper that returns a 16-byte sockaddr. That cuts a 28-byte sockaddr_in6 short, so the bind used whatever followed on the stack. On the current develop image, bind: "::" and bind: "::1" bound nothing. The flow input now builds a sockaddr_storage with uv_ip4_addr/uv_ip6_addr and calls bind(const sockaddr &). sFlow samples from an IPv6 peer now carry an IPv6 sender. Before, the callback parsed every sender as IPv4, so it left those bytes uninitialized. An IPv4-mapped sender from a dual-stack :: bind is kept as IPv4.
Clear refusals: a bind that is not an IPv4 or IPv6 address (localhost), or a port outside 1–65535, is refused with a message that says so. Before, it ran on an ephemeral port, a truncated port, or no port at all.
No exceptions on the loop thread: the server sockets' error handlers, and the dnstap connection handlers, run on the loop thread, where a throw terminates the process. They now log instead. A dnstap client whose accept() or read() fails is closed.

Tests

All 25 ctest targets pass on macOS (Release). The new tests:

flow: a second listener on a port in use fails to start; ::1 receives a valid sFlow datagram and the sample's sender is IPv6 ::1; localhost, port 0 and port 70000 are refused. All but the IPv6 test failed on develop. The IPv6 test passed on develop here only by chance: the truncated address happened to bind at -O2 on macOS, while Linux left it unbound.
dnstap: a TCP server on a port in use, and a Unix socket that can't be bound, fail to start. Both failed on develop.
policies: Bad Config: a flow policy on a port another flow input listens on is refused. Two policies sit on a config-less flow tap, like orb-agent's managed fleet_flow, on one port with different configs. The second load fails and is rolled back, and the first keeps its listener. It failed on develop.

Not changed here

Halted dnstap server: after a failed accept(), or a client handle that can't be created, libuv stops watching the listening socket, so the dnstap server accepts no further clients. Both failures are close to impossible in practice.
Older dnstap and netprobe issues are left for follow-ups:
- a second dnstap Unix input on the same path takes the socket file over instead of failing;
- dnstap's close_event handlers look a session up by the fd of a closing handle;
- netprobe's TCP probe connects through the same truncating uvw helper.

🤖 Generated with Claude Code

…of running without it uvw 3 returns libuv errors from bind(), listen() and recv() rather than emitting them, and the flow and dnstap inputs ignored the return values. A second flow policy whose input listened on a UDP port already in use was accepted and reported running, on a socket that never got the port, so it received nothing. The same held for a dnstap TCP server on a port in use and a Unix socket that could not be bound. start() now checks each call and throws with libuv's reason, after closing the handles and the loop it had created, so the policy load fails and is rolled back. The flow input builds its bind address itself: uvw's bind(ip, port) cut an IPv6 address down to a 16-byte sockaddr, so an IPv6 listener bound whatever the stack held, or nothing. A bind that is not an IPv4 or IPv6 address, or a port outside 1-65535, is refused with a message that says so. The server sockets' error and connection handlers run on the loop thread, where a throw terminates the process. They now log instead, and a client that cannot be accepted or read is closed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

leoparente · 2026-10-29T00:58:55Z

@codex review

leoparente · 2026-10-29T00:58:57Z

/ai-review

chatgpt-codex-connector · 2026-10-29T00:59:09Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-29T01:22:37.179028Z	`285d6fa`	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

nbl-ai-review · 2026-10-29T00:59:13Z

AI Code Review — risk tier: Full · dimensions run: correctness, security

Advisory only. A human owns the merge decision.

Summary — Net positive. The fix correctly makes flow/dnstap listeners fail loudly on bind errors instead of silently running unbound, and the IPv6 sockaddr-truncation fix for flow UDP binding is sound and well-tested. No new issues introduced by this diff; two pre-existing bugs elsewhere are worth tracking separately.

Pre-existing / out-of-scope

src/inputs/flow/FlowInputStream.cpp:216 — The sFlow handler always treats sample.sourceIP as IPv4 and calls inet_pton(AF_INET, ...) on the sender address regardless of actual family. If the sender is IPv6 (or the string doesn't parse), inet_pton fails without initializing peer4.sin_addr, and uninitialized stack bytes are then copied into the recorded source IP (CWE-457). This bug predates the PR, but the PR's IPv6 bind fix makes the flow listener actually reachable over IPv6 for the first time, so this path becomes practically triggerable. Suggest tracking a follow-up to branch on sender address family and zero-initialize peer4/treat parse failure as skip rather than copy.
src/inputs/dnstap/DnstapInputStream.cpp:132 — dnstap's TCP bind path still uses the same truncating bind(host, port) uvw helper this PR replaced for flow UDP, and the tcp config string is still split on the first :, which breaks IPv6 literals like "::1:5354". Not introduced here and not in the PR's listed follow-ups; worth tracking alongside this fix since it's the same class of bug.

No other findings.

_{🤖 AI Code Review · run · prompts: code-review/v1 · models: claude-sonnet-5/medium · context: description · 3 comment(s) · 0 prior AI review(s) · usage: $1.51 · 2616k in (94% cached) · 16.6k out}

nbl-ai-review · 2026-10-29T00:59:26Z

AI Code Review — risk tier: Full · dimensions run: correctness, security

Advisory only. A human owns the merge decision.

Summary — Net positive. Listeners now fail loudly on bind errors instead of running unbound, the IPv6 sockaddr-truncation fix for flow UDP binding is sound, and the sFlow source-IP handling now correctly branches on the sender's address family. No new issues found.

Resolved since last review

src/inputs/flow/FlowInputStream.cpp:216 — sFlow sourceIP handler previously assumed IPv4 and could copy uninitialized stack memory for IPv6 senders; now branches on actual sender address family and zero-initializes appropriately, verified by the new IPv6 sFlow test.

_{🤖 AI Code Review · run · prompts: code-review/v1 · models: claude-sonnet-5/medium · context: description · 7 comment(s) · 2 prior AI review(s) · 1/1 thread(s) resolved · usage: $1.14 · 1786k in (92% cached) · 8.3k out}

Previous review · 2026-10-29 · f8ef6ef

AI Code Review — risk tier: Full · dimensions run: correctness, security

Advisory only. A human owns the merge decision.

Summary — Net-positive reliability fix: bind/listen/accept/read return codes are now checked and surfaced, failed listeners roll back the policy instead of silently running on a dead socket, the IPv6 sockaddr_storage fix addresses a real stack-corruption bug, and the new port/address validation and tests match the stated intent. No correctness or security issues found.

_{🤖 AI Code Review · run · prompts: code-review/v1 · models: claude-sonnet-5/medium · context: description · 3 comment(s) · 0 prior AI review(s) · usage: $1.33 · 3031k in (97% cached) · 11k out}

github-actions · 2026-10-29T01:11:44Z

LCOV of commit `285d6fa` during Debug Builds #263

  lines......: 86.0% (18320 of 21296 lines)
  functions..: 75.8% (1841 of 2428 functions)
  branches...: no data found

Files changed coverage rate: n/a

Full coverage report

leoparente · 2026-10-29T01:11:53Z

@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f8ef6efedc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

With IPv6 binds now working, an sFlow datagram from an IPv6 peer reached the callback, which still parsed the sender as IPv4 only. The failed parse left the sample's sourceIP as uninitialized IPv4 bytes. The sender is now read as IPv4 or IPv6, and an IPv4-mapped address from a dual-stack bind is kept as IPv4. The IPv6 test sends a valid sFlow datagram and checks the sample's sender. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

leoparente · 2026-10-29T01:20:46Z

@codex review

leoparente · 2026-10-29T01:20:48Z

/ai-review

nbl-ai-review · 2026-10-29T01:21:21Z

🔁 AI Code Review updated for 285d6fa4 — see the review · 1 resolved · 0 open (run)

chatgpt-codex-connector · 2026-10-29T01:22:35Z

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: 285d6fa4a3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector Bot reviewed Oct 29, 2026

View reviewed changes

Comment thread src/inputs/flow/FlowInputStream.cpp Resolved

leoparente marked this pull request as ready for review October 29, 2026 12:45

samiura approved these changes Oct 29, 2026

View reviewed changes

leoparente merged commit 68b42d1 into develop Oct 29, 2026
23 checks passed

leoparente deleted the fix/flow-listener-bind-failure branch October 29, 2026 13:59

Conversation

leoparente commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Tests

Not changed here

Uh oh!

leoparente commented Oct 29, 2026

Uh oh!

leoparente commented Oct 29, 2026

Uh oh!

chatgpt-codex-connector Bot commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

nbl-ai-review Bot commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nbl-ai-review Bot commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LCOV of commit 285d6fa during Debug Builds #263

Uh oh!

leoparente commented Oct 29, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

leoparente commented Oct 29, 2026

Uh oh!

leoparente commented Oct 29, 2026

Uh oh!

nbl-ai-review Bot commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Oct 29, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

leoparente commented Oct 29, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Oct 29, 2026 •

edited

Loading

nbl-ai-review Bot commented Oct 29, 2026 •

edited

Loading

nbl-ai-review Bot commented Oct 29, 2026 •

edited

Loading

github-actions Bot commented Oct 29, 2026 •

edited

Loading

LCOV of commit `285d6fa` during Debug Builds #263

nbl-ai-review Bot commented Oct 29, 2026 •

edited

Loading