-
Notifications
You must be signed in to change notification settings - Fork 0
Simplify neverssl FAQ entry #328
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,41 @@ | ||||||||||
| { pkgs }: | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing platform guard. This service uses { pkgs }:
let
inherit (pkgs) lib;
in
lib.mkIf pkgs.stdenv.isLinux {
# ... rest of service definition
} |
||||||||||
| let | ||||||||||
| keepaliveScript = pkgs.writeShellApplication { | ||||||||||
|
Comment on lines
+1
to
+3
|
||||||||||
| name = "neverssl-keepalive"; | ||||||||||
| runtimeInputs = [ pkgs.curl ]; | ||||||||||
| text = '' | ||||||||||
| set -euo pipefail | ||||||||||
| if ! curl -fsS --max-time 10 http://neverssl.com > /dev/null 2>&1; then | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error handling logic appears inverted. With
|
||||||||||
| exit 0 | ||||||||||
| fi | ||||||||||
|
Comment on lines
+7
to
+10
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The shell script logic can be simplified. The current |
||||||||||
| ''; | ||||||||||
| }; | ||||||||||
| in | ||||||||||
| { | ||||||||||
| systemd.user.services.neverssl-keepalive = { | ||||||||||
| Unit = { | ||||||||||
| Description = "Keep captive portal alive via neverssl.com"; | ||||||||||
| Wants = [ "network-online.target" ]; | ||||||||||
| After = [ "network-online.target" ]; | ||||||||||
| }; | ||||||||||
| Service = { | ||||||||||
| Type = "oneshot"; | ||||||||||
| ExecStart = "${keepaliveScript}/bin/neverssl-keepalive"; | ||||||||||
| }; | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| systemd.user.timers.neverssl-keepalive = { | ||||||||||
| Unit = { | ||||||||||
| Description = "Timer for neverssl captive portal keepalive"; | ||||||||||
| }; | ||||||||||
| Timer = { | ||||||||||
| OnBootSec = "3s"; | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A 3-second interval is extremely aggressive for a captive portal keepalive, generating ~28,800 requests per day. This creates unnecessary load on neverssl.com infrastructure, wastes network bandwidth and battery, and is much shorter than typical captive portal timeouts (5-30 minutes). Consider using a 5-10 minute interval instead: OnBootSec = "5min";
OnUnitActiveSec = "5min";Or make it configurable via a module option if users have different timeout needs. |
||||||||||
| OnUnitActiveSec = "3s"; | ||||||||||
|
Comment on lines
+32
to
+33
|
||||||||||
| OnBootSec = "3s"; | |
| OnUnitActiveSec = "3s"; | |
| OnBootSec = "30s"; | |
| OnUnitActiveSec = "30s"; |
Copilot
AI
Nov 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Timer.Unit field is redundant in systemd timers. By default, a timer automatically activates a service with the same name (minus the .timer suffix). This line can be safely removed as neverssl-keepalive.timer will automatically trigger neverssl-keepalive.service.
| Unit = "neverssl-keepalive.service"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Expose configuration options for the service.
The module hardcodes the interval (3s) and URL without exposing any configuration parameters. This violates the coding guidelines requirement that service configurations must document service parameters.
Consider restructuring the module to expose options:
{ config, lib, pkgs, ... }:
with lib;
let
cfg = config.services.neverssl-keepalive;
keepaliveScript = pkgs.writeShellApplication {
name = "neverssl-keepalive";
runtimeInputs = [ pkgs.curl ];
text = ''
set -euo pipefail
if ! curl -fsS --max-time 10 ${cfg.url} > /dev/null 2>&1; then
exit 0
fi
'';
};
in
{
options.services.neverssl-keepalive = {
enable = mkEnableOption "neverssl captive portal keepalive";
interval = mkOption {
type = types.str;
default = "30s";
description = "Interval between keepalive requests";
};
url = mkOption {
type = types.str;
default = "http://neverssl.com";
description = "URL to request for keepalive";
};
};
config = mkIf cfg.enable {
systemd.user.services.neverssl-keepalive = {
Unit = {
Description = "Keep captive portal alive via neverssl.com";
Wants = [ "network-online.target" ];
After = [ "network-online.target" ];
};
Service = {
Type = "oneshot";
ExecStart = "${keepaliveScript}/bin/neverssl-keepalive";
};
};
systemd.user.timers.neverssl-keepalive = {
Unit = {
Description = "Timer for neverssl captive portal keepalive";
};
Timer = {
OnBootSec = cfg.interval;
OnUnitActiveSec = cfg.interval;
AccuracySec = "1s";
Unit = "neverssl-keepalive.service";
};
Install = {
WantedBy = [ "timers.target" ];
};
};
};
}This would require updating home-manager/services/default.nix to pass config and lib parameters.
Based on coding guidelines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The title "Auto renew neverssl on private wifi" could be misleading. Captive portals are most common on public Wi-Fi networks (e.g., airports, hotels), not private ones. Using a more general title would better reflect the feature's purpose.