Skip to content

Open DNS discovery ports - #32

Merged
efrecon merged 3 commits into
masterfrom
feature/discovery-ports
Sep 2, 2026
Merged

efrecon merged 3 commits into
masterfrom
feature/discovery-ports

Conversation

@efrecon

@efrecon efrecon commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Automatically open NetBIOS and mDNS ports when a firewall is installed and running.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

It can open firewall ports even when the announce daemon isn’t actually present/usable, and the new firewall-detection/port-parsing logic lacks test coverage in the existing shellspec suite.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds automatic firewall port opening for DNS discovery/announcement methods so that mDNS and NetBIOS advertisements work when a supported firewall is installed and running.

Changes:

  • Extend the announce step to open mDNS (5353/udp) and NetBIOS (137/udp, 138/udp) ports during install when an active firewall is detected.
  • Introduce firewall detection (ufw, firewalld, nftables) and a generic primer_net_port_allow helper to apply port rules.
File summaries
File Description
libexec/steps/announce.sh Adds per-method port lists and calls into primer_net_port_allow during install when a firewall is active.
libexec/net.sh Implements active firewall detection and port opening across ufw/firewalld/nftables.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread libexec/steps/announce.sh
Comment thread libexec/net.sh
@efrecon
efrecon merged commit 4b1bd16 into master Sep 2, 2026
1 check passed
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.

2 participants