feat: kyber Tailscale exit node with vpn fish shorthand - #1432
Conversation
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a fish shell Changes
Sequence DiagramsequenceDiagram
participant User
participant FishShell as Fish Shell
participant VPNFunc as _vpn_function
participant Tailscale as Tailscale CLI
participant Host as kyber Host
User->>FishShell: run `vpn` / `vpn on|off|status`
FishShell->>VPNFunc: invoke _vpn_function
alt Toggle (no arg)
VPNFunc->>Tailscale: tailscale status --json 2>/dev/null
Tailscale-->>VPNFunc: JSON output
alt ExitNodeStatus present
VPNFunc->>Tailscale: sudo tailscale set --exit-node=
Tailscale-->>VPNFunc: cleared
VPNFunc-->>User: "VPN disconnected"
else ExitNodeStatus absent
VPNFunc->>Tailscale: sudo tailscale set --exit-node=100.74.174.97
Tailscale-->>VPNFunc: set to kyber
VPNFunc-->>User: "VPN connected through kyber"
end
else On
VPNFunc->>Tailscale: sudo tailscale set --exit-node=100.74.174.97
Tailscale-->>User: success
else Off
VPNFunc->>Tailscale: sudo tailscale set --exit-node=
Tailscale-->>User: success
else Status
VPNFunc->>Tailscale: tailscale status
Tailscale-->>User: display status
end
Note right of Host: kyber sysctl changes + --advertise-exit-node ensure Host can act as exit node
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Mesa DescriptionTL;DRConfigured Kyber as a Tailscale exit node and introduced a What changed?
Description generated by Mesa. Update settings |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a vpn function for the Fish shell to manage Tailscale exit nodes and configures the kyber host to act as an exit node by enabling IP forwarding. Feedback focuses on fixing the toggle logic in the Fish function, which currently fails to correctly parse Tailscale's JSON output, adding $DRY_RUN_CMD support to the Nix activation script for safer dry runs, and replacing a hardcoded IP address with a dynamic lookup.
| set -l current (tailscale status --json 2>/dev/null | string match -rg '"ExitNodeStatus"') | ||
| if test -n "$current" | ||
| sudo tailscale set --exit-node= | ||
| echo "VPN disconnected" | ||
| else | ||
| sudo tailscale set --exit-node=$kyber_ip | ||
| echo "VPN connected through kyber" | ||
| end |
There was a problem hiding this comment.
The toggle logic is likely broken. string match -rg '"ExitNodeStatus"' matches the key name in the JSON output even if its value is null (which Tailscale returns when no exit node is active). Consequently, test -n "$current" will always be true, and the function will always attempt to disconnect rather than toggle.
A more robust check is to look for the opening brace of the object value, indicating an active exit node configuration.
if tailscale status --json 2>/dev/null | string match -q '*"ExitNodeStatus": {*'
sudo tailscale set --exit-node=
echo "VPN disconnected"
else
sudo tailscale set --exit-node=$kyber_ip
echo "VPN connected through kyber"
end
| home.activation.enableIpForwarding = config.lib.dag.entryAfter [ "writeBoundary" ] '' | ||
| if [ "$(cat /proc/sys/net/ipv4/ip_forward)" != "1" ]; then | ||
| echo "Enabling IP forwarding for Tailscale exit node..." | ||
| sudo sysctl -w net.ipv4.ip_forward=1 | ||
| sudo sysctl -w net.ipv6.conf.all.forwarding=1 | ||
| fi | ||
| if [ ! -f /etc/sysctl.d/99-tailscale.conf ]; then | ||
| echo 'net.ipv4.ip_forward=1' | sudo tee /etc/sysctl.d/99-tailscale.conf | ||
| echo 'net.ipv6.conf.all.forwarding=1' | sudo tee -a /etc/sysctl.d/99-tailscale.conf | ||
| fi | ||
| ''; |
There was a problem hiding this comment.
This activation script is missing $DRY_RUN_CMD, which is standard for home-manager activation scripts in this repository (see lines 68, 74). Without it, a dry-run (e.g., home-manager build) will still attempt to execute sudo commands.
Also, the check for /etc/sysctl.d/99-tailscale.conf only verifies existence. It's safer to use $DRY_RUN_CMD with sudo and ensure the content is correctly applied.
home.activation.enableIpForwarding = config.lib.dag.entryAfter [ "writeBoundary" ] ''
if [ "$(cat /proc/sys/net/ipv4/ip_forward)" != "1" ]; then
echo "Enabling IP forwarding for Tailscale exit node..."
$DRY_RUN_CMD sudo sysctl -w net.ipv4.ip_forward=1
$DRY_RUN_CMD sudo sysctl -w net.ipv6.conf.all.forwarding=1
fi
if [ ! -f /etc/sysctl.d/99-tailscale.conf ]; then
echo 'net.ipv4.ip_forward=1' | $DRY_RUN_CMD sudo tee /etc/sysctl.d/99-tailscale.conf > /dev/null
echo 'net.ipv6.conf.all.forwarding=1' | $DRY_RUN_CMD sudo tee -a /etc/sysctl.d/99-tailscale.conf > /dev/null
fi
'';
| @@ -0,0 +1,25 @@ | |||
| function _vpn_function --description "Connect/disconnect Tailscale exit node through kyber" | |||
| set -l kyber_ip 100.74.174.97 | |||
There was a problem hiding this comment.
The IP address for kyber is hardcoded. Consider retrieving it dynamically (e.g., tailscale ip -4 kyber) or making it a configurable variable to improve maintainability.
There was a problem hiding this comment.
Pull request overview
Adds a Tailscale exit-node setup for the kyber host and introduces a fish shorthand (vpn) to control using kyber as an exit node from interactive fish shells.
Changes:
- Advertises
kyberas a Tailscale exit node and adds a Home Manager activation hook to enable IP forwarding. - Adds
_vpn_functionfish function and wiresvpnas a fish abbreviation. - Adds fishtape tests for the new fish function.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
named-hosts/kyber/default.nix |
Enables IP forwarding via activation and adds --advertise-exit-node to Tailscale up args. |
home-manager/programs/fish/functions/_vpn_function.fish |
Implements vpn on/off/status (and a toggle default) for Tailscale exit-node usage. |
home-manager/programs/fish/default.nix |
Registers vpn abbreviation and ships _vpn_function into fish functions. |
spec/fish/_vpn_function_test.fish |
Adds initial fishtape tests for the new fish function. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if [ "$(cat /proc/sys/net/ipv4/ip_forward)" != "1" ]; then | ||
| echo "Enabling IP forwarding for Tailscale exit node..." | ||
| sudo sysctl -w net.ipv4.ip_forward=1 | ||
| sudo sysctl -w net.ipv6.conf.all.forwarding=1 | ||
| fi | ||
| if [ ! -f /etc/sysctl.d/99-tailscale.conf ]; then | ||
| echo 'net.ipv4.ip_forward=1' | sudo tee /etc/sysctl.d/99-tailscale.conf | ||
| echo 'net.ipv6.conf.all.forwarding=1' | sudo tee -a /etc/sysctl.d/99-tailscale.conf |
There was a problem hiding this comment.
Activation script runs privileged commands (sysctl/tee) without using $DRY_RUN_CMD or the repo’s existing root-command helper pattern (see modules/tailscale install hook). This means home-manager switch --dry-run could still modify sysctl settings, and activation will hard-fail/prompt if sudo isn’t available or is interactive. Prefer wrapping all mutating commands with $DRY_RUN_CMD and using a resolved root helper (sudo/doas/root) like the tailscale module does, or move sysctl config to a system-level module instead of home activation.
| if [ "$(cat /proc/sys/net/ipv4/ip_forward)" != "1" ]; then | |
| echo "Enabling IP forwarding for Tailscale exit node..." | |
| sudo sysctl -w net.ipv4.ip_forward=1 | |
| sudo sysctl -w net.ipv6.conf.all.forwarding=1 | |
| fi | |
| if [ ! -f /etc/sysctl.d/99-tailscale.conf ]; then | |
| echo 'net.ipv4.ip_forward=1' | sudo tee /etc/sysctl.d/99-tailscale.conf | |
| echo 'net.ipv6.conf.all.forwarding=1' | sudo tee -a /etc/sysctl.d/99-tailscale.conf | |
| if [ "$(id -u)" -eq 0 ]; then | |
| root_cmd="" | |
| elif command -v sudo >/dev/null 2>&1; then | |
| root_cmd="sudo" | |
| elif command -v doas >/dev/null 2>&1; then | |
| root_cmd="doas" | |
| else | |
| echo "Unable to enable IP forwarding: need root privileges but neither sudo nor doas is available." >&2 | |
| exit 1 | |
| fi | |
| run_root() { | |
| if [ -n "$root_cmd" ]; then | |
| $DRY_RUN_CMD "$root_cmd" "$@" | |
| else | |
| $DRY_RUN_CMD "$@" | |
| fi | |
| } | |
| if [ "$(cat /proc/sys/net/ipv4/ip_forward)" != "1" ]; then | |
| echo "Enabling IP forwarding for Tailscale exit node..." | |
| run_root sysctl -w net.ipv4.ip_forward=1 | |
| run_root sysctl -w net.ipv6.conf.all.forwarding=1 | |
| fi | |
| if [ ! -f /etc/sysctl.d/99-tailscale.conf ]; then | |
| printf '%s\n' 'net.ipv4.ip_forward=1' | run_root tee /etc/sysctl.d/99-tailscale.conf >/dev/null | |
| printf '%s\n' 'net.ipv6.conf.all.forwarding=1' | run_root tee -a /etc/sysctl.d/99-tailscale.conf >/dev/null |
| extraUpArgs = [ | ||
| "--reset" | ||
| "--accept-dns=false" | ||
| "--advertise-exit-node" | ||
| ]; |
There was a problem hiding this comment.
--advertise-exit-node is being added via extraUpArgs even though the tailscale module already exposes a first-class modules.tailscale.advertiseExitNode option. Using the option is clearer, avoids accidental duplication if args/options are later combined, and keeps intent declarative.
| # Toggle: if exit node is set, turn off; otherwise turn on | ||
| set -l current (tailscale status --json 2>/dev/null | string match -rg '"ExitNodeStatus"') | ||
| if test -n "$current" | ||
| sudo tailscale set --exit-node= | ||
| echo "VPN disconnected" | ||
| else | ||
| sudo tailscale set --exit-node=$kyber_ip | ||
| echo "VPN connected through kyber" | ||
| end |
There was a problem hiding this comment.
Toggle mode (case '') attempts to detect whether an exit node is set by regex-matching the literal JSON key name "ExitNodeStatus". This doesn’t actually inspect a value (e.g., null vs object, or which exit node is selected), so the toggle decision is fragile and may be incorrect depending on the JSON shape. Parse a specific field/value (and ideally confirm it matches the intended kyber exit node) before deciding on/off.
| # Toggle: if exit node is set, turn off; otherwise turn on | |
| set -l current (tailscale status --json 2>/dev/null | string match -rg '"ExitNodeStatus"') | |
| if test -n "$current" | |
| sudo tailscale set --exit-node= | |
| echo "VPN disconnected" | |
| else | |
| sudo tailscale set --exit-node=$kyber_ip | |
| echo "VPN connected through kyber" | |
| end | |
| # Toggle: turn off only if the current exit node is kyber; otherwise turn on. | |
| set -l status_json (tailscale status --json 2>/dev/null) | |
| if test $status -ne 0 | |
| echo "Unable to determine Tailscale status" | |
| return 1 | |
| end | |
| echo "$status_json" | python3 -c ' | |
| import json | |
| import sys | |
| kyber_ip = sys.argv[1] | |
| try: | |
| data = json.load(sys.stdin) | |
| except Exception: | |
| sys.exit(2) | |
| exit_node_status = data.get("ExitNodeStatus") | |
| tailscale_ips = [] | |
| if isinstance(exit_node_status, dict): | |
| ips = exit_node_status.get("TailscaleIPs") | |
| if isinstance(ips, list): | |
| tailscale_ips = [str(ip) for ip in ips] | |
| sys.exit(0 if kyber_ip in tailscale_ips else 1) | |
| ' "$kyber_ip" | |
| switch $status | |
| case 0 | |
| sudo tailscale set --exit-node= | |
| echo "VPN disconnected" | |
| case 1 | |
| sudo tailscale set --exit-node=$kyber_ip | |
| echo "VPN connected through kyber" | |
| case '*' | |
| echo "Unable to parse Tailscale status" | |
| return 1 | |
| end |
| function tailscale; echo "tailscale $argv"; end | ||
|
|
||
| @test "function is defined after sourcing" (functions -q _vpn_function; echo $status) = 0 | ||
| @test "vpn status calls tailscale status" (_vpn_function status) = "tailscale status" |
There was a problem hiding this comment.
The new fish tests only cover sourcing and the status branch. Since _vpn_function introduces side-effecting behavior (on, off, and toggle default case) plus an error path, add tests that stub sudo/tailscale and assert the expected commands/output for each branch to prevent regressions.
| @test "vpn status calls tailscale status" (_vpn_function status) = "tailscale status" | |
| @test "vpn status calls tailscale status" (_vpn_function status) = "tailscale status" | |
| @test "vpn on calls sudo tailscale up" (_vpn_function on) = "sudo tailscale up" | |
| @test "vpn off calls sudo tailscale down" (_vpn_function off) = "sudo tailscale down" | |
| @test "vpn with no args uses toggle/default branch" (_vpn_function) = "tailscale status" | |
| @test "vpn invalid arg returns usage/error output" (_vpn_function invalid) = "usage: vpn [on|off|status]" |
| if [ "$(cat /proc/sys/net/ipv4/ip_forward)" != "1" ]; then | ||
| echo "Enabling IP forwarding for Tailscale exit node..." | ||
| sudo sysctl -w net.ipv4.ip_forward=1 | ||
| sudo sysctl -w net.ipv6.conf.all.forwarding=1 | ||
| fi |
There was a problem hiding this comment.
The guard only checks /proc/sys/net/ipv4/ip_forward before setting both IPv4 and IPv6 forwarding. If IPv4 forwarding is already enabled but IPv6 forwarding is still disabled, this block won’t run and IPv6 forwarding won’t be enabled as intended. Consider checking both sysctls (or unconditionally applying both) so the IPv6 setting isn’t skipped.
| echo "Enabling IP forwarding for Tailscale exit node..." | ||
| sudo sysctl -w net.ipv4.ip_forward=1 | ||
| sudo sysctl -w net.ipv6.conf.all.forwarding=1 | ||
| fi | ||
| if [ ! -f /etc/sysctl.d/99-tailscale.conf ]; then | ||
| echo 'net.ipv4.ip_forward=1' | sudo tee /etc/sysctl.d/99-tailscale.conf | ||
| echo 'net.ipv6.conf.all.forwarding=1' | sudo tee -a /etc/sysctl.d/99-tailscale.conf | ||
| fi |
There was a problem hiding this comment.
Persistence step only runs when /etc/sysctl.d/99-tailscale.conf does not exist. If the file exists but is missing one of the forwarding keys (or has them set to 0), activation won’t correct it. Consider ensuring the desired contents (e.g., write/replace after a compare) rather than a simple existence check.
| echo "Enabling IP forwarding for Tailscale exit node..." | |
| sudo sysctl -w net.ipv4.ip_forward=1 | |
| sudo sysctl -w net.ipv6.conf.all.forwarding=1 | |
| fi | |
| if [ ! -f /etc/sysctl.d/99-tailscale.conf ]; then | |
| echo 'net.ipv4.ip_forward=1' | sudo tee /etc/sysctl.d/99-tailscale.conf | |
| echo 'net.ipv6.conf.all.forwarding=1' | sudo tee -a /etc/sysctl.d/99-tailscale.conf | |
| fi | |
| echo "Enabling IPv4 forwarding for Tailscale exit node..." | |
| sudo sysctl -w net.ipv4.ip_forward=1 | |
| fi | |
| if [ "$(cat /proc/sys/net/ipv6/conf/all/forwarding)" != "1" ]; then | |
| echo "Enabling IPv6 forwarding for Tailscale exit node..." | |
| sudo sysctl -w net.ipv6.conf.all.forwarding=1 | |
| fi | |
| tmp_sysctl_file="$(mktemp)" | |
| cat > "$tmp_sysctl_file" <<'EOF' | |
| net.ipv4.ip_forward=1 | |
| net.ipv6.conf.all.forwarding=1 | |
| EOF | |
| if [ ! -f /etc/sysctl.d/99-tailscale.conf ] || ! cmp -s "$tmp_sysctl_file" /etc/sysctl.d/99-tailscale.conf; then | |
| echo "Updating persistent IP forwarding configuration for Tailscale exit node..." | |
| sudo install -Dm644 "$tmp_sysctl_file" /etc/sysctl.d/99-tailscale.conf | |
| fi | |
| rm -f "$tmp_sysctl_file" |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
home-manager/programs/fish/functions/_vpn_function.fish (1)
1-2: Avoid hardcoding kyber's Tailscale IP in a second place.
100.74.174.97is already duplicated inhome-manager/programs/ssh/default.nix. If kyber is rekeyed or rebuilt, the SSH config and VPN helper can drift. Please lift this into one shared source of truth.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@home-manager/programs/fish/functions/_vpn_function.fish` around lines 1 - 2, Avoid duplicating the hardcoded IP in _vpn_function: remove the literal 100.74.174.97 assignment to the local kyber_ip and instead read the value from the single shared source of truth (e.g., an environment variable or fish_user_variable that your home-manager/Nix config sets). Update _vpn_function to use that external variable (referencing kyber_ip as read from $KYBER_TAILSCALE_IP or from fish_user_variables) so the VPN helper and SSH config both derive the kyber Tailscale IP from the same central configuration.spec/fish/_vpn_function_test.fish (1)
7-8: Cover the publicvpnentrypoint too.These specs still pass if the
vpnwiring inhome-manager/programs/fish/default.nixis removed or renamed, because they source the private helper directly. Please add an assertion for the exportedvpncommand path as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/fish/_vpn_function_test.fish` around lines 7 - 8, The tests currently only source the private helper _vpn_function so they pass even if the public vpn entrypoint is missing; update spec/fish/_vpn_function_test.fish to also assert the exported command exists and forwards correctly by adding an assertion that the public function is defined (invoke `functions -q vpn` and check exit status is 0) and a test that calling the public entrypoint (e.g., `(vpn status)`) returns the same output as the helper (e.g., `"tailscale status"`), referencing the existing `_vpn_function` and `vpn` names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@home-manager/programs/fish/default.nix`:
- Line 162: The current assignment sets vpn to an abbreviation (_vpn_function)
which isn't callable from scripts; change this to expose a real function by
either renaming the existing _vpn_function to vpn (so the function symbol is
actually "vpn") or adding a thin wrapper function file (e.g., vpn.fish) that
calls _vpn_function, and update the declaration so the exported name is "vpn"
instead of "_vpn_function"; ensure the function symbol vpn is defined in the
fish functions list so commands like `fish -c 'vpn status'` succeed.
In `@named-hosts/kyber/default.nix`:
- Around line 150-159: The activation script in
home.activation.enableIpForwarding only guards on IPv4 (the cat
/proc/sys/net/ipv4/ip_forward check) which leaves IPv6 forwarding unaddressed
and the /etc/sysctl.d/99-tailscale.conf file unchanged if it exists but is
stale; update the shell snippet inside home.activation.enableIpForwarding to
test and set both net.ipv4.ip_forward and net.ipv6.conf.all.forwarding
independently (e.g., check each /proc/sys value and call sysctl -w for each when
needed) and ensure the persistent file /etc/sysctl.d/99-tailscale.conf is
created or repaired idempotently (replace or merge the two settings into the
file rather than only appending when the file is missing) so enabling forwarding
is correct for both IPv4 and IPv6 and repeated runs are safe.
---
Nitpick comments:
In `@home-manager/programs/fish/functions/_vpn_function.fish`:
- Around line 1-2: Avoid duplicating the hardcoded IP in _vpn_function: remove
the literal 100.74.174.97 assignment to the local kyber_ip and instead read the
value from the single shared source of truth (e.g., an environment variable or
fish_user_variable that your home-manager/Nix config sets). Update _vpn_function
to use that external variable (referencing kyber_ip as read from
$KYBER_TAILSCALE_IP or from fish_user_variables) so the VPN helper and SSH
config both derive the kyber Tailscale IP from the same central configuration.
In `@spec/fish/_vpn_function_test.fish`:
- Around line 7-8: The tests currently only source the private helper
_vpn_function so they pass even if the public vpn entrypoint is missing; update
spec/fish/_vpn_function_test.fish to also assert the exported command exists and
forwards correctly by adding an assertion that the public function is defined
(invoke `functions -q vpn` and check exit status is 0) and a test that calling
the public entrypoint (e.g., `(vpn status)`) returns the same output as the
helper (e.g., `"tailscale status"`), referencing the existing `_vpn_function`
and `vpn` names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 87e052a6-d831-4dc9-9466-26c4e87d0111
📒 Files selected for processing (4)
home-manager/programs/fish/default.nixhome-manager/programs/fish/functions/_vpn_function.fishnamed-hosts/kyber/default.nixspec/fish/_vpn_function_test.fish
| two = "_two_function"; | ||
| tzo = "_tzo_function"; | ||
| ulb = "_update_local_binaries_function"; | ||
| vpn = "_vpn_function"; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In fish shell, do abbreviations configured via Home Manager programs.fish.shellAbbrs become callable commands/functions, or are they only interactive editor expansions?
💡 Result:
In fish shell, abbreviations configured via Home Manager's programs.fish.shellAbbrs are interactive editor expansions, not callable commands or functions. They are added to ~/.config/fish/config.fish using abbr --add commands, which register them for expansion during interactive input (e.g., typing the abbr and pressing space or enter expands it). They do not create executable commands; running the abbr name directly treats it as an unknown command unless it matches an existing command or function. Fish official documentation confirms: "Only typed-in commands use abbreviations. Abbreviations are not expanded in scripts." Home Manager tests verify abbr --add generation in config.fish, confirming interactive setup.
Citations:
- 1: https://github.com/nix-community/home-manager/blob/d166a078/tests/modules/programs/fish/abbrs.nix
- 2: https://fishshell.com/docs/current/cmds/abbr.html
- 3: https://github.com/nix-community/home-manager/blob/master/modules/programs/fish.nix
- 4: fish: source hm-session-vars.sh only for interactive shells nix-community/home-manager#3539
vpn needs to be a real function, not an abbreviation.
The abbreviation at line 162 only expands during interactive input; fish -c 'vpn status' and scripts will fail with "unknown command." Fish abbreviations are not callable commands. Either create a thin vpn.fish wrapper function or rename _vpn_function to vpn to make it actually callable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@home-manager/programs/fish/default.nix` at line 162, The current assignment
sets vpn to an abbreviation (_vpn_function) which isn't callable from scripts;
change this to expose a real function by either renaming the existing
_vpn_function to vpn (so the function symbol is actually "vpn") or adding a thin
wrapper function file (e.g., vpn.fish) that calls _vpn_function, and update the
declaration so the exported name is "vpn" instead of "_vpn_function"; ensure the
function symbol vpn is defined in the fish functions list so commands like `fish
-c 'vpn status'` succeed.
| home.activation.enableIpForwarding = config.lib.dag.entryAfter [ "writeBoundary" ] '' | ||
| if [ "$(cat /proc/sys/net/ipv4/ip_forward)" != "1" ]; then | ||
| echo "Enabling IP forwarding for Tailscale exit node..." | ||
| sudo sysctl -w net.ipv4.ip_forward=1 | ||
| sudo sysctl -w net.ipv6.conf.all.forwarding=1 | ||
| fi | ||
| if [ ! -f /etc/sysctl.d/99-tailscale.conf ]; then | ||
| echo 'net.ipv4.ip_forward=1' | sudo tee /etc/sysctl.d/99-tailscale.conf | ||
| echo 'net.ipv6.conf.all.forwarding=1' | sudo tee -a /etc/sysctl.d/99-tailscale.conf | ||
| fi |
There was a problem hiding this comment.
Make the forwarding activation idempotent for IPv6 too.
The guard on Line 151 only checks IPv4. If net.ipv4.ip_forward=1 is already set but net.ipv6.conf.all.forwarding=0, this block skips both writes and the host is still advertised as an exit node with incomplete forwarding. Lines 156-159 also never repair an existing but stale /etc/sysctl.d/99-tailscale.conf.
Suggested fix
home.activation.enableIpForwarding = config.lib.dag.entryAfter [ "writeBoundary" ] ''
- if [ "$(cat /proc/sys/net/ipv4/ip_forward)" != "1" ]; then
- echo "Enabling IP forwarding for Tailscale exit node..."
- sudo sysctl -w net.ipv4.ip_forward=1
- sudo sysctl -w net.ipv6.conf.all.forwarding=1
- fi
- if [ ! -f /etc/sysctl.d/99-tailscale.conf ]; then
- echo 'net.ipv4.ip_forward=1' | sudo tee /etc/sysctl.d/99-tailscale.conf
- echo 'net.ipv6.conf.all.forwarding=1' | sudo tee -a /etc/sysctl.d/99-tailscale.conf
- fi
+ if [ "$(cat /proc/sys/net/ipv4/ip_forward)" != "1" ]; then
+ sudo sysctl -w net.ipv4.ip_forward=1
+ fi
+ if [ "$(cat /proc/sys/net/ipv6/conf/all/forwarding)" != "1" ]; then
+ sudo sysctl -w net.ipv6.conf.all.forwarding=1
+ fi
+ cat <<'EOF' | sudo tee /etc/sysctl.d/99-tailscale.conf >/dev/null
+net.ipv4.ip_forward=1
+net.ipv6.conf.all.forwarding=1
+EOF
'';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@named-hosts/kyber/default.nix` around lines 150 - 159, The activation script
in home.activation.enableIpForwarding only guards on IPv4 (the cat
/proc/sys/net/ipv4/ip_forward check) which leaves IPv6 forwarding unaddressed
and the /etc/sysctl.d/99-tailscale.conf file unchanged if it exists but is
stale; update the shell snippet inside home.activation.enableIpForwarding to
test and set both net.ipv4.ip_forward and net.ipv6.conf.all.forwarding
independently (e.g., check each /proc/sys value and call sysctl -w for each when
needed) and ensure the persistent file /etc/sysctl.d/99-tailscale.conf is
created or repaired idempotently (replace or merge the two settings into the
file rather than only appending when the file is missing) so enabling forwarding
is correct for both IPv4 and IPv6 and repeated runs are safe.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="named-hosts/kyber/default.nix">
<violation number="1" location="named-hosts/kyber/default.nix:151">
P1: IPv6 forwarding may remain disabled because the condition only checks IPv4 state before applying both sysctl updates.</violation>
<violation number="2" location="named-hosts/kyber/default.nix:153">
P2: Missing `$DRY_RUN_CMD` before `sudo` commands. Other activation scripts in this file use `$DRY_RUN_CMD` to prevent privileged commands from running during `home-manager build` / `--dry-run`. Without it, a dry-run will still execute `sudo sysctl` and `sudo tee`.</violation>
<violation number="3" location="named-hosts/kyber/default.nix:156">
P2: The sysctl persistence logic is not self-healing: existing but incomplete/incorrect config files are never corrected.</violation>
</file>
<file name="home-manager/programs/fish/functions/_vpn_function.fish">
<violation number="1" location="home-manager/programs/fish/functions/_vpn_function.fish:13">
P1: Toggle logic is broken: `string match -rg '"ExitNodeStatus"'` matches the JSON key name itself, which is present in the output even when the value is `null` (no exit node active). This means `test -n "$current"` is always true, so the toggle will always disconnect and never connect. Match against the value shape instead, e.g. `string match -q '*"ExitNodeStatus": {*'` to detect an active exit node object.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| # IP forwarding for Tailscale exit node | ||
| home.activation.enableIpForwarding = config.lib.dag.entryAfter [ "writeBoundary" ] '' | ||
| if [ "$(cat /proc/sys/net/ipv4/ip_forward)" != "1" ]; then |
There was a problem hiding this comment.
P1: IPv6 forwarding may remain disabled because the condition only checks IPv4 state before applying both sysctl updates.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At named-hosts/kyber/default.nix, line 151:
<comment>IPv6 forwarding may remain disabled because the condition only checks IPv4 state before applying both sysctl updates.</comment>
<file context>
@@ -146,6 +146,19 @@ home-manager.lib.homeManagerConfiguration {
+ # IP forwarding for Tailscale exit node
+ home.activation.enableIpForwarding = config.lib.dag.entryAfter [ "writeBoundary" ] ''
+ if [ "$(cat /proc/sys/net/ipv4/ip_forward)" != "1" ]; then
+ echo "Enabling IP forwarding for Tailscale exit node..."
+ sudo sysctl -w net.ipv4.ip_forward=1
</file context>
| if [ "$(cat /proc/sys/net/ipv4/ip_forward)" != "1" ]; then | |
| if [ "$(cat /proc/sys/net/ipv4/ip_forward)" != "1" ] || [ "$(cat /proc/sys/net/ipv6/conf/all/forwarding)" != "1" ]; then |
| tailscale status | ||
| case '' | ||
| # Toggle: if exit node is set, turn off; otherwise turn on | ||
| set -l current (tailscale status --json 2>/dev/null | string match -rg '"ExitNodeStatus"') |
There was a problem hiding this comment.
P1: Toggle logic is broken: string match -rg '"ExitNodeStatus"' matches the JSON key name itself, which is present in the output even when the value is null (no exit node active). This means test -n "$current" is always true, so the toggle will always disconnect and never connect. Match against the value shape instead, e.g. string match -q '*"ExitNodeStatus": {*' to detect an active exit node object.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/programs/fish/functions/_vpn_function.fish, line 13:
<comment>Toggle logic is broken: `string match -rg '"ExitNodeStatus"'` matches the JSON key name itself, which is present in the output even when the value is `null` (no exit node active). This means `test -n "$current"` is always true, so the toggle will always disconnect and never connect. Match against the value shape instead, e.g. `string match -q '*"ExitNodeStatus": {*'` to detect an active exit node object.</comment>
<file context>
@@ -0,0 +1,25 @@
+ tailscale status
+ case ''
+ # Toggle: if exit node is set, turn off; otherwise turn on
+ set -l current (tailscale status --json 2>/dev/null | string match -rg '"ExitNodeStatus"')
+ if test -n "$current"
+ sudo tailscale set --exit-node=
</file context>
| sudo sysctl -w net.ipv4.ip_forward=1 | ||
| sudo sysctl -w net.ipv6.conf.all.forwarding=1 | ||
| fi | ||
| if [ ! -f /etc/sysctl.d/99-tailscale.conf ]; then |
There was a problem hiding this comment.
P2: The sysctl persistence logic is not self-healing: existing but incomplete/incorrect config files are never corrected.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At named-hosts/kyber/default.nix, line 156:
<comment>The sysctl persistence logic is not self-healing: existing but incomplete/incorrect config files are never corrected.</comment>
<file context>
@@ -146,6 +146,19 @@ home-manager.lib.homeManagerConfiguration {
+ sudo sysctl -w net.ipv4.ip_forward=1
+ sudo sysctl -w net.ipv6.conf.all.forwarding=1
+ fi
+ if [ ! -f /etc/sysctl.d/99-tailscale.conf ]; then
+ echo 'net.ipv4.ip_forward=1' | sudo tee /etc/sysctl.d/99-tailscale.conf
+ echo 'net.ipv6.conf.all.forwarding=1' | sudo tee -a /etc/sysctl.d/99-tailscale.conf
</file context>
| if [ ! -f /etc/sysctl.d/99-tailscale.conf ]; then | |
| if [ ! -f /etc/sysctl.d/99-tailscale.conf ] || ! sudo grep -q '^net.ipv4.ip_forward=1$' /etc/sysctl.d/99-tailscale.conf || ! sudo grep -q '^net.ipv6.conf.all.forwarding=1$' /etc/sysctl.d/99-tailscale.conf; then |
| home.activation.enableIpForwarding = config.lib.dag.entryAfter [ "writeBoundary" ] '' | ||
| if [ "$(cat /proc/sys/net/ipv4/ip_forward)" != "1" ]; then | ||
| echo "Enabling IP forwarding for Tailscale exit node..." | ||
| sudo sysctl -w net.ipv4.ip_forward=1 |
There was a problem hiding this comment.
P2: Missing $DRY_RUN_CMD before sudo commands. Other activation scripts in this file use $DRY_RUN_CMD to prevent privileged commands from running during home-manager build / --dry-run. Without it, a dry-run will still execute sudo sysctl and sudo tee.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At named-hosts/kyber/default.nix, line 153:
<comment>Missing `$DRY_RUN_CMD` before `sudo` commands. Other activation scripts in this file use `$DRY_RUN_CMD` to prevent privileged commands from running during `home-manager build` / `--dry-run`. Without it, a dry-run will still execute `sudo sysctl` and `sudo tee`.</comment>
<file context>
@@ -146,6 +146,19 @@ home-manager.lib.homeManagerConfiguration {
+ home.activation.enableIpForwarding = config.lib.dag.entryAfter [ "writeBoundary" ] ''
+ if [ "$(cat /proc/sys/net/ipv4/ip_forward)" != "1" ]; then
+ echo "Enabling IP forwarding for Tailscale exit node..."
+ sudo sysctl -w net.ipv4.ip_forward=1
+ sudo sysctl -w net.ipv6.conf.all.forwarding=1
+ fi
</file context>
Summary
--advertise-exit-node)vpnfish function (vpn on/vpn off/vpn status)Usage
Summary by cubic
Turned kyber into a Tailscale exit node and added a
vpnfish command (vpn on|off|status) to route all traffic through kyber. Also enables IP forwarding on kyber, registers the function for autoloading, and adds basic tests.--advertise-exit-node) and enable IPv4/IPv6 forwarding viahome-manageractivation; persists settings in/etc/sysctl.d/99-tailscale.conf._vpn_function.fishwired asvpn; supportson,off,status, and toggle; targets kyber100.74.174.97; registered in functions mapping/list for reliable autoloading.vpn statuscallstailscale status.Written for commit 6672f24. Summary will update on new commits.