feat(groq): A whimsical concurrent network scanner that treats host:port pairs as radio frequencies and reports which ones emit a signal.

[polsala]

Implementation Summary

• Utility: nightly-signal-scanner
• Provider: groq
• Location: go-utils/nightly-nightly-signal-scanner
• Files Created: 3
• Description: A whimsical concurrent network scanner that treats host:port pairs as radio frequencies and reports which ones emit a signal.

Rationale

• Automated proposal from the Groq generator delivering a fresh community utility.
• This utility was generated using the groq AI provider.

Why safe to merge

• Utility is isolated to go-utils/nightly-nightly-signal-scanner.
• README + tests ship together (see folder contents).
• No secrets or credentials touched.
• All changes are additive and self-contained.

Test Plan

• Follow the instructions in the generated README at go-utils/nightly-nightly-signal-scanner/README.md
• Run tests located in go-utils/nightly-nightly-signal-scanner/tests/

Links

• Generated docs and examples committed alongside this change.

Mock Justification

• Not applicable; generator did not introduce new mocks.

[polsala]

🤖 Review by GROQ Agent

✅ What's solid

• Clear purpose & isolation – The scanner lives in its own go-utils/nightly-nightly-signal-scanner folder, keeping the rest of the repo untouched.
• Concurrency handling – Uses a semaphore channel (sem) together with a sync.WaitGroup to limit parallel connections, which is a simple and reliable pattern.
• Graceful stdin handling – Trims empty lines and reports read errors to stderr before exiting with a non‑zero status.
• Configurable flags – Timeout (-t) and concurrency (-c) are exposed via flag, with sensible defaults.
• Testable design – The dialFunc indirection makes the core networking logic mockable without pulling in heavy test frameworks.

🧪 Tests

• Coverage – The single unit test exercises the happy‑path and a failure case, confirming that runScanner returns the expected messages.
• Potential improvements
  • Edge‑case inputs – Add tests for:
    • Empty address strings.
    • Addresses with surrounding whitespace.
    • Very large input slices to verify the semaphore correctly throttles concurrency.
  • Timeout behavior – Simulate a dial that blocks longer than the supplied timeout to ensure the timeout is respected (e.g., using a custom dialFunc that sleeps).
  • Order preservation – The current implementation preserves input order; a test that shuffles the mock responses can guard against accidental changes that might reorder results.

func TestRunScanner_Timeout(t *testing.T) {
    original := dialFunc
    defer func() { dialFunc = original }()

    dialFunc = func(_, _ string, timeout time.Duration) (net.Conn, error) {
        // Simulate a long‑running dial that exceeds the timeout.
        time.Sleep(timeout + 10*time.Millisecond)
        return nil, fmt.Errorf("timeout")
    }

    results := runScanner([]string{"slow.com:1234"}, 5*time.Millisecond, 1)
    if !strings.Contains(results[0], "No signal") {
        t.Fatalf("expected timeout to be reported as no signal, got %q", results[0])
    }
}

🔒 Security

• No credential leakage – The utility only opens outbound TCP connections; it does not read or write any secrets.
• Input validation – Currently the code trusts the host:port format supplied via stdin. Consider:
  • Validating that the port is numeric and within 1‑65535.
  • Rejecting or sanitizing malformed lines to avoid accidental DNS lookups on malformed host strings.
• Resource exhaustion – While the semaphore caps concurrent connections, an attacker could still feed an extremely large list causing the program to allocate a huge slice (results). Adding a hard limit (e.g., maxLines := 10_000) with a clear error message would mitigate memory pressure.

🧩 Docs/DX

• README completeness – The README explains installation, usage, and flags, which is great for a CLI tool.
• Suggested enhancements
  • Example with a file – Show piping from a file (cat hosts.txt | ./signal‑scanner) to illustrate typical workflow.
  • Exit codes – Document that the program always exits 0 unless stdin reading fails; consider returning a non‑zero code when any host fails, which can be useful in CI pipelines.
  • Cross‑platform note – Mention that the binary requires Go 1.22+ and works on Linux/macOS/Windows (if tested).
  • Help output – The flag package automatically prints usage, but adding a -h/--help example in the README can guide users.

🧱 Mocks/Fakes

• dialFunc abstraction – Well‑implemented; the test replaces it with a mock that returns a net.Pipe connection, avoiding real network calls.
• Further mock opportunities
  • Inject a logger – Replace direct fmt.Println/fmt.Fprintln with a logger interface; tests could then capture output without relying on stdout.
  • Context support – Refactor checkPort to accept a context.Context (or at least a deadline) so that cancellation can be tested and the tool can be stopped gracefully (e.g., via Ctrl‑C).

Overall, the contribution is clean, functional, and well‑structured. Adding a few edge‑case tests, tightening input validation, and documenting exit semantics will make the utility more robust and production‑ready.

[polsala]

🤖 Review by GEMINI Agent

✅ What's solid

• The utility provides a clear and concise function: scanning host:port pairs for network connectivity.
• The "whimsical" theme is well-integrated into the README and output messages, adding a touch of personality.
• The use of bufio.Scanner for reading from STDIN is idiomatic Go.
• Concurrency is handled effectively using goroutines and a semaphore (chan struct{}) to limit the number of simultaneous connections, preventing resource exhaustion.
• The net.DialTimeout function is correctly used to enforce connection timeouts.
• The code is well-structured with a main function and a runScanner helper function.
• Error handling for reading from STDIN is present.

🧪 Tests

• The existing test covers a basic scenario with one "successful" and one "failed" connection.
• Consider adding tests for edge cases:
  • Empty input: What happens if STDIN is empty? The current code would likely produce an empty output slice, which is probably fine, but worth verifying.
  • Invalid host:port format: While the net.DialTimeout might handle some of this, explicitly testing an invalid format like invalid-host or host:port:extra could be beneficial.
  • Zero timeout: Test how the scanner behaves with a timeout of 0 seconds.
  • Zero concurrency: Test with -c 0. The current implementation would likely block indefinitely as the semaphore channel would have a capacity of 0.
  • Large number of inputs: While not strictly a unit test, consider how performance scales with a very large number of inputs.
• The mock implementation in test_main.go is good, but it relies on a specific hardcoded address ("good.com:80"). It would be more robust to make the mock more flexible, perhaps by allowing it to be configured with a map of addresses to success/failure.

🔒 Security

• The utility does not handle any secrets or credentials, which is a significant security positive.
• The primary risk would be denial-of-service if the tool were misused to scan an excessive number of ports on a target, but this is inherent to network scanning tools and not a specific vulnerability in this implementation.
• Ensure that the net.DialTimeout function itself is not susceptible to any known vulnerabilities related to network probing.

🧩 Docs/DX

• The README is clear, well-organized, and provides excellent instructions for building and usage.
• The "Why 'Signal'?" section adds a nice thematic touch.
• The example usage is practical and easy to follow.
• The flag descriptions are informative.
• Consider adding a note about the Go version requirement (1.22+) in the installation section, as mentioned in the README.
• It would be beneficial to explicitly state the default values for the flags in the Flags section of the README for quick reference.

🧱 Mocks/Fakes

• The approach of making dialFunc a package-level variable that can be reassigned for testing is a common and effective pattern for mocking in Go.
• The current mock in test_main.go is functional for its intended purpose.
• As mentioned in the "Tests" section, consider enhancing the mock to be more configurable. For instance, a map could be used to define the behavior for different addresses:

// Example of a more configurable mock
var mockDialResponses = map[string]error{
    "good.com:80": nil,
    "another.com:443": nil,
    "bad.com:1234": fmt.Errorf("mock failure"),
}

dialFunc = func(network, address string, timeout time.Duration) (net.Conn, error) {
    if err, ok := mockDialResponses[address]; ok {
        if err == nil {
            c1, c2 := net.Pipe()
            c2.Close()
            return c1, nil
        }
        return nil, err
    }
    // Default behavior for unconfigured addresses
    return nil, fmt.Errorf("mock failure: unconfigured address %s", address)
}