Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions home-manager/services/neverssl-keepalive/keepalive.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,15 @@ set -euo pipefail

# Silently ping neverssl.com - ignore failures (network may be unavailable)
curl -fsS --max-time 10 http://neverssl.com >/dev/null 2>&1 || true

# macOS-specific: Open captive portal for Starbucks WiFi if connectivity is lost
if [[ $OSTYPE == "darwin"* ]]; then
SSID=$(networksetup -getairportnetwork en0 2>/dev/null | awk -F": " '{print $2}' || echo "")

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.

high

The Wi-Fi network interface is hardcoded as en0. While this is common on many Macs, it's not guaranteed. Other interfaces like en1 could be used, especially on older hardware or systems with multiple network adapters. To make this script more robust, you should dynamically determine the Wi-Fi interface device name.

The suggestion below also refactors the awk command to use -v FS=... which is a bit cleaner and avoids potential shell quoting issues.

Suggested change
SSID=$(networksetup -getairportnetwork en0 2>/dev/null | awk -F": " '{print $2}' || echo "")
SSID=$(networksetup -getairportnetwork "$(networksetup -listallhardwareports | awk '/Hardware Port: (Wi-Fi|AirPort)/{getline; print $2}')" 2>/dev/null | awk -v FS=": " '{print $2}' || echo "")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Hardcode the WiFi interface name or detect it dynamically.

The hardcoded en0 interface may not work on all macOS systems; the device name could be airport, en0, en1, etc, depending on the Mac hardware and the version of OS X. Consider detecting the WiFi interface dynamically using networksetup -listallhardwareports or making it configurable.

🤖 Prompt for AI Agents
In home-manager/services/neverssl-keepalive/keepalive.sh around line 12 the WiFi
interface is hardcoded as en0 which fails on machines where the WiFi device uses
a different name; change the script to determine the active WiFi interface
dynamically or make it configurable: run networksetup -listallhardwareports and
parse the "Wi-Fi"/"AirPort"/"WiFi" Hardware Port entry to get the Device value
(fall back to common names like en0/en1), assign that to a variable (e.g.
WIFI_IFACE) and use it instead of en0, and allow overriding via an environment
variable or config option so the script works across macOS variants.

Copilot AI Dec 21, 2025

Copy link

Choose a reason for hiding this comment

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

The network interface name is hardcoded to en0, but macOS systems may have different primary WiFi interface names (e.g., en1, en2) depending on hardware configuration. Consider making this more robust by detecting the active WiFi interface dynamically, or document this assumption if en0 is guaranteed to be the WiFi interface in your target environment.

Suggested change
SSID=$(networksetup -getairportnetwork en0 2>/dev/null | awk -F": " '{print $2}' || echo "")
WIFI_DEVICE=$(networksetup -listallhardwareports 2>/dev/null | awk '/Wi-Fi/{getline; if ($1=="Device:") {print $2; exit}}')
: "${WIFI_DEVICE:=en0}"
SSID=$(networksetup -getairportnetwork "$WIFI_DEVICE" 2>/dev/null | awk -F": " '{print $2}' || echo "")

Copilot uses AI. Check for mistakes.

# Check for Starbucks networks (e.g., at_STARBUCKS_Wi2)
if [[ $SSID == *"STARBUCKS"* ]]; then

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.

high

The pattern *"STARBUCKS"* includes literal double quotes, so it would only match an SSID that contains "STARBUCKS" including the quotes, which is unlikely. The quotes should be removed from the pattern.

Additionally, the comparison is case-sensitive. A case-insensitive match would be more robust and catch variations like Starbucks or starbucks.

The suggested change addresses both issues.

Suggested change
if [[ $SSID == *"STARBUCKS"* ]]; then
if [[ ${SSID,,} == *starbucks* ]]; then

if ! ping -c 1 -W 2 1.1.1.1 >/dev/null 2>&1; then

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.

critical

The ping command's -W flag behaves differently on macOS compared to Linux. On macOS, it specifies the timeout in milliseconds, not seconds. A timeout of 2 milliseconds is far too short and will likely cause the ping to fail even with a good connection, leading to the captive portal opening unnecessarily on every script execution.

You should use the -t flag for a timeout in seconds on macOS, or increase the value for -W to 2000 for a 2-second timeout.

Suggested change
if ! ping -c 1 -W 2 1.1.1.1 >/dev/null 2>&1; then
if ! ping -c 1 -t 2 1.1.1.1 >/dev/null 2>&1; then

@cubic-dev-ai cubic-dev-ai Bot Dec 21, 2025

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.

P1: On macOS, ping -W expects time in milliseconds, not seconds. -W 2 sets a 2ms timeout, which is too short for any realistic network response and will almost always fail, causing the captive portal to open unnecessarily even when connectivity is working.

Use -W 2000 for a 2-second timeout, or use -t 2 which specifies timeout in seconds.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/neverssl-keepalive/keepalive.sh, line 16:

<comment>On macOS, `ping -W` expects time in **milliseconds**, not seconds. `-W 2` sets a 2ms timeout, which is too short for any realistic network response and will almost always fail, causing the captive portal to open unnecessarily even when connectivity is working.

Use `-W 2000` for a 2-second timeout, or use `-t 2` which specifies timeout in seconds.</comment>

<file context>
@@ -6,3 +6,15 @@ set -euo pipefail
+
+  # Check for Starbucks networks (e.g., at_STARBUCKS_Wi2)
+  if [[ $SSID == *&quot;STARBUCKS&quot;* ]]; then
+    if ! ping -c 1 -W 2 1.1.1.1 &gt;/dev/null 2&gt;&amp;1; then
+      open &quot;http://captive.apple.com&quot; 2&gt;/dev/null || true
+    fi
</file context>
Suggested change
if ! ping -c 1 -W 2 1.1.1.1 >/dev/null 2>&1; then
if ! ping -c 1 -W 2000 1.1.1.1 >/dev/null 2>&1; then
Fix with Cubic

open "http://captive.apple.com" 2>/dev/null || true
fi
fi
fi
23 changes: 23 additions & 0 deletions spec/keepalive_spec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,27 @@ The output should include 'captive portal'
End
End

Describe 'Starbucks WiFi detection (macOS)'
It 'checks for macOS via OSTYPE'
When run bash -c "cat '$SCRIPT'"
The output should include 'OSTYPE'
The output should include 'darwin'
End

It 'uses networksetup to get SSID'
When run bash -c "cat '$SCRIPT'"
The output should include 'networksetup -getairportnetwork'
End

It 'checks for STARBUCKS SSID pattern'
When run bash -c "cat '$SCRIPT'"
The output should include '*"STARBUCKS"*'
End

It 'opens captive portal when connectivity fails'
When run bash -c "cat '$SCRIPT'"
The output should include 'captive.apple.com'
End
Comment on lines +87 to +106

Copilot AI Dec 21, 2025

Copy link

Choose a reason for hiding this comment

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

These tests only verify that certain strings exist in the script source code, but they don't validate the actual behavior or logic flow. Consider adding behavioral tests that mock the commands (networksetup, ping, open) and verify the script executes correctly under different conditions, similar to the existing "curl behavior" and "error handling" test suites. For example, test scenarios where: (1) SSID contains "STARBUCKS" and ping fails, (2) SSID doesn't contain "STARBUCKS", (3) SSID contains "STARBUCKS" but ping succeeds, and (4) networksetup command fails.

Copilot uses AI. Check for mistakes.
End
Comment on lines +86 to +107

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.

medium

The new tests for Starbucks WiFi detection only check the script's source code for specific strings. This makes them brittle and tightly coupled to implementation details. For example, if the string matching for "STARBUCKS" is improved (e.g., to be case-insensitive as I suggested elsewhere), these tests would fail.

The existing tests in this file for curl behavior use mocking (mock_bin_setup) to test the script's actual behavior under different conditions. It would be better to follow that pattern for the new tests as well.

For example, you could:

  1. Mock networksetup to return a Starbucks SSID.
  2. Mock ping to fail.
  3. Assert that open "http://captive.apple.com" is called.

And another test where ping succeeds, and assert that open is not called. This would provide much more confidence in the correctness of the logic.


End
Loading