Skip to content

Conversation

@jhaynie
Copy link
Member

@jhaynie jhaynie commented Oct 21, 2025

Summary by CodeRabbit

  • New Features

    • Detect listening TCP ports on macOS and Linux, with optional exclusion; results are unique and returned in ascending order, covering IPv4/IPv6/system-specific sources and resilient to missing system data.
  • Tests

    • Added comprehensive tests validating detection, exclusion behavior, sorting, duplicate prevention, actual listener detection, and port range validation.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 21, 2025

Walkthrough

Adds a cross-platform exported function DetectListeningTCPPorts(exclude ...int) to the network package with Darwin and Linux implementations and a test suite; each implementation enumerates listening TCP ports, applies exclusions, and returns a sorted, unique slice of ports.

Changes

Cohort / File(s) Summary
Platform implementations
network/port_darwin.go, network/port_linux.go
Adds DetectListeningTCPPorts(exclude ...int) ([]int, error) with OS-specific logic: Darwin runs lsof -nP -iTCP -sTCP:LISTEN and parses output via regex; Linux reads /proc/net/tcp and /proc/net/tcp6, selects LISTEN entries (state 0A), parses hex ports. Both build an exclusion set, deduplicate, sort, and return ports; handle command/file errors appropriately.
Tests
network/port_test.go
Adds tests validating retrieval, ascending order, single/multiple exclusions, non-existent exclusions, detection of an actual bound listener via net.Listen, exclusion of a bound listener, duplicate suppression, and valid port range checks (1–65535).

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant C as Caller
    participant N as network.DetectListeningTCPPorts
    rect rgb(240,248,255)
    Note right of N: Platform dispatch (build tags)
    end
    C->>N: DetectListeningTCPPorts(exclude...)
    alt Darwin build
        N->>OS: run `lsof -nP -iTCP -sTCP:LISTEN`
        OS-->>N: command stdout (lines)
        N->>N: regex parse ports, filter exclude, dedupe, sort
    else Linux build
        N->>FS: open `/proc/net/tcp` and `/proc/net/tcp6`
        FS-->>N: file contents
        N->>N: parse entries, filter state==0A, extract hex port, filter exclude, dedupe, sort
    end
    N-->>C: ([]int, error)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 I sniff the pipes where bytes do play,
I peek with lsof and proc all day,
I skip the ports you tell me not to see,
I sort and prune them—neat as can be.
Hoppity hops, ports found—one, two, three!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "add a utility for finding all the TCP ports in listen (with the ability to exclude ports)" directly and accurately describes the primary change in the changeset. The PR introduces a new exported function DetectListeningTCPPorts with platform-specific implementations for Darwin and Linux that enables detection of listening TCP ports with optional exclusion capability. The title is clear, specific, and free of vague terminology, accurately conveying the main objective of the change to someone reviewing the commit history.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch network-ports

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 673522b and a6fbfc0.

📒 Files selected for processing (3)
  • network/port_darwin.go (1 hunks)
  • network/port_linux.go (1 hunks)
  • network/port_test.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • network/port_darwin.go
🧰 Additional context used
🧬 Code graph analysis (2)
network/port_linux.go (1)
network/port_darwin.go (1)
  • DetectListeningTCPPorts (18-61)
network/port_test.go (2)
network/port_linux.go (1)
  • DetectListeningTCPPorts (16-73)
network/port_darwin.go (1)
  • DetectListeningTCPPorts (18-61)
🔇 Additional comments (3)
network/port_linux.go (2)

22-23: Good fix: Using map for deduplication.

The implementation now correctly uses map[int]struct{} to collect unique ports, which addresses the previous concern about duplicate ports appearing when the same port is listening on both IPv4 and IPv6. This aligns with the Darwin implementation pattern.


67-72: LGTM: Correct conversion and sorting.

The conversion from the deduplicated map to a sorted slice is implemented correctly. The explicit sort ensures deterministic, ascending order regardless of map iteration order.

network/port_test.go (1)

11-129: Excellent test coverage.

The test suite is comprehensive and well-structured:

  • Properly uses require.NoError for critical checks and assert for validations
  • Covers core functionality, exclusion logic, edge cases, duplicate prevention, and range validation
  • Appropriately uses t.Skip when environment conditions aren't met
  • Tests actual listener detection using ephemeral ports, which validates real-world behavior
  • Clean resource management with defer listener.Close()

The tests will run correctly on both Darwin and Linux platforms.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
network/port_linux.go (1)

32-65: Consider checking scanner errors for consistency.

The Darwin implementation checks scanner.Err() after the scanning loop, but the Linux version does not. While scanning errors from /proc/net/tcp* are unlikely, checking for consistency and defensive coding would be prudent.

After the loop closes the file (and before moving to the next file), add:

		f.Close()
		
		if err := scanner.Err(); err != nil {
			// Log or handle scanner error if needed
			// For now, continue to next file
			continue
		}
	}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 12df85a and 673522b.

📒 Files selected for processing (3)
  • network/port_darwin.go (1 hunks)
  • network/port_linux.go (1 hunks)
  • network/port_test.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
network/port_darwin.go (1)
network/port_linux.go (1)
  • DetectListeningTCPPorts (16-69)
network/port_linux.go (1)
network/port_darwin.go (1)
  • DetectListeningTCPPorts (18-61)
network/port_test.go (2)
network/port_darwin.go (1)
  • DetectListeningTCPPorts (18-61)
network/port_linux.go (1)
  • DetectListeningTCPPorts (16-69)
🔇 Additional comments (2)
network/port_darwin.go (1)

18-61: LGTM: Solid implementation with proper deduplication.

The Darwin implementation correctly:

  • Uses a map to deduplicate ports (multiple processes can listen on the same port)
  • Checks scanner.Err() for scanning errors
  • Sorts results consistently
  • Handles exclusions properly
network/port_test.go (1)

11-129: LGTM: Comprehensive test coverage.

The test suite provides excellent coverage:

  • Basic functionality and sorting
  • Single and multiple exclusions
  • Non-existent exclusions (edge case)
  • Actual listener detection (integration-style)
  • Duplicate prevention (lines 110-119 will catch the Linux implementation bug)
  • Valid port range validation

The tests appropriately use t.Skip() when insufficient ports are available, making them resilient to different system states.

Comment on lines +25 to +30
for _, path := range files {
f, err := os.Open(path)
if err != nil {
continue // skip if file doesn't exist or can't be read
}
defer f.Close()
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix resource leak: defer in loop.

The defer f.Close() executes at function exit, not at the end of each loop iteration. If both /proc/net/tcp and /proc/net/tcp6 exist, the first file remains open until the function returns.

Apply this diff to close immediately after reading:

 	for _, path := range files {
 		f, err := os.Open(path)
 		if err != nil {
 			continue // skip if file doesn't exist or can't be read
 		}
-		defer f.Close()
 
 		scanner := bufio.NewScanner(f)
 		// Skip the header line
 		if scanner.Scan() {
 			for scanner.Scan() {
 				fields := strings.Fields(scanner.Text())
 				if len(fields) < 4 {
 					continue
 				}
 
 				localAddr := fields[1]
 				state := fields[3]
 				if state != "0A" { // 0A means LISTEN
 					continue
 				}
 
 				// localAddr example: 0100007F:1F90
 				parts := strings.Split(localAddr, ":")
 				if len(parts) != 2 {
 					continue
 				}
 
 				portHex := parts[1]
 				portDec, err := strconv.ParseInt(portHex, 16, 32)
 				if err != nil {
 					continue
 				}
 				port := int(portDec)
 
 				if _, excluded := excludeSet[port]; !excluded {
 					ports = append(ports, port)
 				}
 			}
 		}
+		f.Close()
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for _, path := range files {
f, err := os.Open(path)
if err != nil {
continue // skip if file doesn't exist or can't be read
}
defer f.Close()
for _, path := range files {
f, err := os.Open(path)
if err != nil {
continue // skip if file doesn't exist or can't be read
}
scanner := bufio.NewScanner(f)
// Skip the header line
if scanner.Scan() {
for scanner.Scan() {
fields := strings.Fields(scanner.Text())
if len(fields) < 4 {
continue
}
localAddr := fields[1]
state := fields[3]
if state != "0A" { // 0A means LISTEN
continue
}
// localAddr example: 0100007F:1F90
parts := strings.Split(localAddr, ":")
if len(parts) != 2 {
continue
}
portHex := parts[1]
portDec, err := strconv.ParseInt(portHex, 16, 32)
if err != nil {
continue
}
port := int(portDec)
if _, excluded := excludeSet[port]; !excluded {
ports = append(ports, port)
}
}
}
f.Close()
}
🤖 Prompt for AI Agents
In network/port_linux.go around lines 25 to 30, the code uses "defer f.Close()"
inside the loop which defers closing until function exit and leaks file
descriptors when iterating multiple files; replace the defer with an explicit
close after the file is read (or wrap the read logic in a short-lived inner
function and defer f.Close() there) so each opened file is closed immediately at
the end of its iteration.

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