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
9 changes: 7 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ setup: nix-setup ## Basic Nix setup (alias for nix-setup).
setup-dev: nix-setup git-submodule-sync shell-install ## Set up local development environment (Nix + submodules + shell).

.PHONY: switch
switch: nix-switch launchctl ## Apply Nix configuration and restart launchd agents.
switch: nix-switch ## Apply Nix configuration and restart launchd agents.

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 comment for the switch target still mentions "restart launchd agents", but the launchctl dependency has been removed. This creates an inconsistency between the target's description and its actual actions.

switch: nix-switch ## Apply Nix configuration.

@cubic-dev-ai cubic-dev-ai Bot Dec 10, 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.

P2: Comment is now misleading: the launchctl dependency was removed but the comment still says "restart launchd agents". Update the comment to reflect the actual behavior.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Makefile, line 138:

<comment>Comment is now misleading: the `launchctl` dependency was removed but the comment still says &quot;restart launchd agents&quot;. Update the comment to reflect the actual behavior.</comment>

<file context>
@@ -135,7 +135,7 @@ setup: nix-setup ## Basic Nix setup (alias for nix-setup).
 
 .PHONY: switch
-switch: nix-switch launchctl ## Apply Nix configuration and restart launchd agents.
+switch: nix-switch ## Apply Nix configuration and restart launchd agents.
 
 .PHONY: test
</file context>
Suggested change
switch: nix-switch ## Apply Nix configuration and restart launchd agents.
switch: nix-switch ## Apply Nix configuration.
Fix with Cubic


.PHONY: test
test: neovim-test
Expand Down Expand Up @@ -320,7 +320,12 @@ nix-switch: ## Activate Nix configuration.
elif [ "$(NIX_CONFIG_TYPE)" = "nixosConfigurations" ]; then \
sudo $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure nixpkgs#nixos-rebuild -- switch --flake .#$(NIX_SYSTEM); \
elif [ "$(NIX_CONFIG_TYPE)" = "homeConfigurations" ]; then \
USER=$(NIX_USERNAME) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure .#$(NIX_CONFIG_TYPE)."$(NIX_USERNAME)@$(NIX_SYSTEM)".activationPackage; \
if [ -n "$(HOST)" ]; then \
echo "Switching named home config: $(HOST)"; \
USER=$(NIX_USERNAME) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure .#homeConfigurations.$(HOST).activationPackage; \
else \
USER=$(NIX_USERNAME) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure .#$(NIX_CONFIG_TYPE)."$(NIX_USERNAME)@$(NIX_SYSTEM)".activationPackage; \
fi; \
Comment on lines +323 to +328

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

In the homeConfigurations block, the if [ -n "$(HOST)" ] branch hardcodes homeConfigurations.$(HOST).activationPackage, while the else branch uses $(NIX_CONFIG_TYPE)."$(NIX_USERNAME)@$(NIX_SYSTEM)".activationPackage. For consistency and to avoid potential issues if NIX_CONFIG_TYPE were to change its value unexpectedly within this context, it would be clearer to use $(NIX_CONFIG_TYPE) in both branches if it's guaranteed to be homeConfigurations at this point, or explicitly state homeConfigurations in both if that's the intent.

if [ -n "$(HOST)" ]; then \
	echo "Switching named home config: $(HOST)"; \
	USER=$(NIX_USERNAME) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure .#$(NIX_CONFIG_TYPE).$(HOST).activationPackage; \
else \
	USER=$(NIX_USERNAME) $(NIX_ALLOW_UNFREE) $(NIX_EXEC) run $(NIX_FLAGS) --impure .#$(NIX_CONFIG_TYPE)."$(NIX_USERNAME)@$(NIX_SYSTEM)".activationPackage; \
fi;

else \
echo "Unsupported OS $(OS) for non-CI switch"; \
exit 1; \
Expand Down
3 changes: 3 additions & 0 deletions bun.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions config/claude/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"Bash(git cherry-pick:*)",
"Bash(git commit:*)",
"Bash(git diff:*)",
"Bash(git fetch:*)",
"Bash(git log:*)",
"Bash(git merge:*)",
"Bash(git pull:*)",
Expand Down
2 changes: 1 addition & 1 deletion config/cliproxyapi/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ port: 8317
remote-management:
# Whether to allow remote (non-localhost) management access.
# When false, only localhost can access management endpoints (a key is still required).
allow-remote: false
allow-remote: true

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.

security-high high

Changing allow-remote to true enables remote management access for cliproxyapi. This is a security-sensitive change that could expose the management API to external networks. Please ensure this is intentional and that appropriate network-level access controls (e.g., firewall rules) are in place to restrict access to trusted sources only. If this is for development or specific internal use, consider adding a prominent warning or making it configurable via environment variables rather than a static true in the default config.

Copilot AI Dec 10, 2025

Copy link

Choose a reason for hiding this comment

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

Enabling allow-remote: true exposes the management API to non-localhost connections. While authentication is still required (line 11 shows secret-key: ""), the empty secret key means the Management API is currently disabled (404 for all routes). If a secret key is later added, this configuration will allow remote access. Ensure this is intentional and that strong authentication is in place before deploying.

Suggested change
allow-remote: true
allow-remote: false

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium

Enabling remote management access significantly expands the attack surface. While line 10 mentions that a management key is required, the actual key configuration is not visible in this diff. Ensure that: 1) A strong management key is configured (not left empty/default), 2) The service is only accessible via Tailscale VPN (not exposed to the public internet), and 3) This change is intentional and documented in the PR description with security implications.

Agent: 🏛 Architecture • Fix in Cursor • Fix in Claude

Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#404
File: config/cliproxyapi/config.yaml#L7
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.

Feedback:
Enabling remote management access significantly expands the attack surface. While line 10 mentions that a management key is required, the actual key configuration is not visible in this diff. Ensure that: 1) A strong management key is configured (not left empty/default), 2) The service is only accessible via Tailscale VPN (not exposed to the public internet), and 3) This change is intentional and documented in the PR description with security implications.

# Management key. If a plaintext value is provided here, it will be hashed on startup.
# All management requests (even from localhost) require this key.
# Leave empty to disable the Management API entirely (404 for all /v0/management routes).
Expand Down
4 changes: 4 additions & 0 deletions config/cliproxyapi/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@
home.file.".cli-proxy-api/config.template.yaml" = {
source = config.lib.file.mkOutOfStoreSymlink ./config.yaml;
};
# Example config - required by cliproxyapi's object-backed config bootstrap
home.file.".cli-proxy-api/config.example.yaml" = {
source = config.lib.file.mkOutOfStoreSymlink ./config.yaml;
};
}
1 change: 1 addition & 0 deletions config/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
./factory
./ghostty
./hammerspoon
./k3s
./karabiner
./opencode
./serena
Expand Down
2 changes: 2 additions & 0 deletions config/k3s/config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
disable:
- traefik
22 changes: 22 additions & 0 deletions config/k3s/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
config,
lib,
pkgs,
...
}:
{
# k3s config file stored in home directory
home.file.".config/k3s/config.yaml" = lib.mkIf pkgs.stdenv.isLinux {
source = config.lib.file.mkOutOfStoreSymlink ./config.yaml;
};

# Activation script to sync config to /etc/rancher/k3s/
home.activation.k3s-config = lib.mkIf pkgs.stdenv.isLinux (
lib.hm.dag.entryAfter [ "writeBoundary" ] ''
if [ -f "$HOME/.config/k3s/config.yaml" ]; then
$DRY_RUN_CMD /usr/bin/sudo mkdir -p /etc/rancher/k3s

@cubic-dev-ai cubic-dev-ai Bot Dec 10, 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.

P2: Consider using more robust sudo detection like the tailscale module does. The hardcoded /usr/bin/sudo path may fail on systems where sudo is located elsewhere (e.g., NixOS), and there's no error handling if the command fails.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/k3s/default.nix, line 17:

<comment>Consider using more robust sudo detection like the tailscale module does. The hardcoded `/usr/bin/sudo` path may fail on systems where sudo is located elsewhere (e.g., NixOS), and there&#39;s no error handling if the command fails.</comment>

<file context>
@@ -0,0 +1,22 @@
+  home.activation.k3s-config = lib.mkIf pkgs.stdenv.isLinux (
+    lib.hm.dag.entryAfter [ &quot;writeBoundary&quot; ] &#39;&#39;
+      if [ -f &quot;$HOME/.config/k3s/config.yaml&quot; ]; then
+        $DRY_RUN_CMD /usr/bin/sudo mkdir -p /etc/rancher/k3s
+        $DRY_RUN_CMD /usr/bin/sudo cp &quot;$HOME/.config/k3s/config.yaml&quot; /etc/rancher/k3s/config.yaml
+      fi
</file context>
Fix with Cubic

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium

The hard-coded /usr/bin/sudo path will fail on systems where sudo is installed elsewhere (e.g., NixOS where it's in /run/wrappers/bin/sudo, or some distributions using /bin/sudo). Use command -v sudo or check multiple paths as done in the Tailscale module (lines 225-236 of home-manager/modules/tailscale/default.nix). Additionally, there's no error handling if the sudo commands fail.

Agent: 🏛 Architecture • Fix in Cursor • Fix in Claude

Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#404
File: config/k3s/default.nix#L17
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.

Feedback:
The hard-coded `/usr/bin/sudo` path will fail on systems where sudo is installed elsewhere (e.g., NixOS where it's in `/run/wrappers/bin/sudo`, or some distributions using `/bin/sudo`). Use `command -v sudo` or check multiple paths as done in the Tailscale module (lines 225-236 of `home-manager/modules/tailscale/default.nix`). Additionally, there's no error handling if the sudo commands fail.

$DRY_RUN_CMD /usr/bin/sudo cp "$HOME/.config/k3s/config.yaml" /etc/rancher/k3s/config.yaml
Comment on lines +17 to +18

Copilot AI Dec 10, 2025

Copy link

Choose a reason for hiding this comment

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

The hardcoded path /usr/bin/sudo may not work on all Linux distributions (e.g., NixOS places sudo in a different location). Consider using just sudo without the full path to let the shell's PATH resolution find it, or follow the pattern used in the Tailscale module (lines 223-237 in home-manager/modules/tailscale/default.nix) which checks for sudo availability before using it.

Suggested change
$DRY_RUN_CMD /usr/bin/sudo mkdir -p /etc/rancher/k3s
$DRY_RUN_CMD /usr/bin/sudo cp "$HOME/.config/k3s/config.yaml" /etc/rancher/k3s/config.yaml
$DRY_RUN_CMD sudo mkdir -p /etc/rancher/k3s
$DRY_RUN_CMD sudo cp "$HOME/.config/k3s/config.yaml" /etc/rancher/k3s/config.yaml

Copilot uses AI. Check for mistakes.
fi
''
);
}
5 changes: 5 additions & 0 deletions flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,11 @@
username = "runner";
system = "aarch64-linux";
};
kyber = import ./named-hosts/kyber {
inherit inputs;
username = "ubuntu";
system = "x86_64-linux";
};
};
};

Expand Down
1 change: 1 addition & 0 deletions home-manager/modules/default.nix
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
[
./npm-globals
./tailscale
./yek
]
277 changes: 277 additions & 0 deletions home-manager/modules/tailscale/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,277 @@
# Tailscale configuration
{
config,
lib,
pkgs,
...
}:
with lib;
let
cfg = config.modules.tailscale;

# Check if user-level services should be enabled
# Only enable if serviceConfig is explicitly set (not for system-level service only)
configEnabled = cfg.serviceConfig != { } && cfg.serviceConfig != null;

# System service unit file (for non-NixOS Linux)
tailscaledServiceFile = pkgs.writeText "tailscaled.service" ''
[Unit]
Description=Tailscale node agent
Documentation=https://tailscale.com/kb/
Wants=network-pre.target
After=network-pre.target NetworkManager.service systemd-resolved.service

[Service]
ExecStartPre=${cfg.tailscaled.package}/bin/tailscaled --cleanup
ExecStart=${cfg.tailscaled.package}/bin/tailscaled --state=/var/lib/tailscale/tailscaled.state --socket=/run/tailscale/tailscaled.sock --port ${toString cfg.port}
ExecStopPost=${cfg.tailscaled.package}/bin/tailscaled --cleanup
Restart=on-failure
RuntimeDirectory=tailscale
RuntimeDirectoryMode=0755
StateDirectory=tailscale
StateDirectoryMode=0700
CacheDirectory=tailscale
CacheDirectoryMode=0750
Type=notify

[Install]
WantedBy=multi-user.target
'';
in
{
options.modules.tailscale = {
enable = mkEnableOption "Tailscale VPN service";

# Tailscale daemon configuration
tailscaled = {
package = mkOption {
type = types.package;
default = pkgs.tailscale;
description = "Tailscale package to use";
};

stateDir = mkOption {
type = types.str;
default = "${config.xdg.dataHome}/tailscale";
description = "Directory for Tailscale state files";
};

socketPath = mkOption {
type = types.str;
default = "${config.home.homeDirectory}/.local/run/tailscale/tailscaled.sock";
description = "Socket path for Tailscale daemon";
};
};

# Optional auth key
authKey = mkOption {
type = types.str;
default = "";
description = "Tailscale auth key (use agenix for secrets)";
};

# Optional auth key file (better for secrets)
authKeyFile = mkOption {
type = types.str;
default = "";
description = "Path to file containing Tailscale auth key (better for secrets)";
};

# Tailscale up options
acceptRoutes = mkOption {
type = types.bool;
default = false;
description = "Accept routes from other nodes";
};

advertiseExitNode = mkOption {
type = types.bool;
default = false;
description = "Advertise as exit node";
};

useExitNode = mkOption {
type = types.str;
default = "";
description = "Use specific node as exit node";
};

extraUpArgs = mkOption {
type = types.listOf types.str;
default = [ ];
description = "Additional arguments to pass to tailscale up";
};

port = mkOption {
type = types.int;
default = 41641;
description = "UDP port for Tailscale traffic";
};

installSystemService = mkOption {
type = types.bool;
default = true;
description = "Install system-level tailscaled service (requires sudo, for non-NixOS Linux)";
};

# Important directories and files
directories = mkOption {
type = types.attrsOf types.anything;
default = { };
description = "Integration with home-manager's directories option";
};

files = mkOption {
type = types.attrsOf types.anything;
default = { };
description = "Integration with home-manager's files option";
};

# Service module
serviceConfig = mkOption {
type = types.attrsOf types.anything;
default = { };
description = "Optional systemd service configuration override";
};
};

config = mkIf cfg.enable {
home.packages = [ cfg.tailscaled.package ];

# Declare directories and files for home-manager
home.file.".local/share/tailscale/tailscaled.state".source =
config.lib.file.mkOutOfStoreSymlink "${config.home.homeDirectory}/.local/state/tailscale/tailscaled.state";

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 tailscaled.state file is configured to be a symlink from .local/share/tailscale/tailscaled.state to ${config.home.homeDirectory}/.local/state/tailscale/tailscaled.state. While xdg.dataHome defaults to ~/.local/share, the target path uses ~/.local/state. This creates an inconsistency in where state data is expected to reside according to XDG Base Directory Specification. It would be clearer and more consistent to either use xdg.stateHome for the target path or keep the state file directly within xdg.dataHome without symlinking to a different XDG directory type.


# Tailscaled service
systemd.user.services.tailscaled = mkIf configEnabled {
Unit = {
Description = "Tailscale VPN daemon";
After = [ "network-online.target" ];
Wants = [ "network-online.target" ];
};

Service = {
ExecStart = "${pkgs.tailscale}/bin/tailscaled --state=${config.xdg.dataHome}/tailscale/tailscaled.state --socket=${cfg.tailscaled.socketPath}";
Restart = "on-failure";
RestartSec = 5;
}
// cfg.serviceConfig.service or { };

Install.WantedBy = [ "default.target" ];
};

# Tailscale up service (runs once to connect)
systemd.user.services.tailscale-up = mkIf configEnabled {
Unit = {
Description = "Connect Tailscale to network";
After = [ "tailscaled.service" ];
Requires = [ "tailscaled.service" ];
};

Service =
let
authKeyArg =
if cfg.authKey != "" then
"--authkey=${cfg.authKey}"

@cubic-dev-ai cubic-dev-ai Bot Dec 10, 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: Auth key passed directly on command line is visible in process listings via ps aux. Consider deprecating the authKey option in favor of authKeyFile only, or use environment variables with EnvironmentFile to pass the secret securely.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/modules/tailscale/default.nix, line 175:

<comment>Auth key passed directly on command line is visible in process listings via `ps aux`. Consider deprecating the `authKey` option in favor of `authKeyFile` only, or use environment variables with `EnvironmentFile` to pass the secret securely.</comment>

<file context>
@@ -0,0 +1,277 @@
+        let
+          authKeyArg =
+            if cfg.authKey != &quot;&quot; then
+              &quot;--authkey=${cfg.authKey}&quot;
+            else if cfg.authKeyFile != &quot;&quot; then
+              &quot;--authkey-file=${cfg.authKeyFile}&quot;
</file context>
Fix with Cubic

else if cfg.authKeyFile != "" then
"--authkey-file=${cfg.authKeyFile}"
else
"";

upArgs =
lib.filter (x: x != "") [
authKeyArg
(optionalString cfg.acceptRoutes "--accept-routes")
(optionalString cfg.advertiseExitNode "--advertise-exit-node")
(optionalString (cfg.useExitNode != "") "--exit-node=${cfg.useExitNode}")
]
++ cfg.extraUpArgs;

upCommand = "${pkgs.tailscale}/bin/tailscale up ${concatStringsSep " " upArgs}";
in
{
Type = "oneshot";
RemainAfterExit = true;
ExecStart = upCommand;
ExecStop = "${pkgs.tailscale}/bin/tailscale down";
};

Install.WantedBy = [ "default.target" ];
};

home.activation.createTailscaleDirs = lib.hm.dag.entryAfter [ "writeBoundary" ] ''
TAILSCALE_STATE_DIR="${config.xdg.dataHome}/tailscale"
TAILSCALE_RUN_DIR="$(dirname "${cfg.tailscaled.socketPath}")"

# Create directories
$DRY_RUN_CMD mkdir -p "$TAILSCALE_STATE_DIR"
$DRY_RUN_CMD mkdir -p "$TAILSCALE_RUN_DIR"

# Set proper permissions
$DRY_RUN_CMD chmod 700 "$TAILSCALE_STATE_DIR"
$DRY_RUN_CMD chmod 700 "$TAILSCALE_RUN_DIR"
'';

# Install system-level tailscaled service (requires sudo)
# Uses nix-generated service file for full declarative config
home.activation.installTailscaleService = mkIf cfg.installSystemService (
lib.hm.dag.entryAfter [ "writeBoundary" ] ''
SERVICE_FILE="/etc/systemd/system/tailscaled.service"
NIX_SERVICE="${tailscaledServiceFile}"
SUDOERS_FILE="/etc/sudoers.d/nix-tailscale"

# Resolve an elevated command helper
SUDO_CMD=""
if command -v sudo >/dev/null 2>&1; then
SUDO_CMD="sudo"
elif [ -x /usr/bin/sudo ]; then
SUDO_CMD="/usr/bin/sudo"
elif command -v doas >/dev/null 2>&1; then
SUDO_CMD="doas"
elif [ -x /usr/bin/doas ]; then
SUDO_CMD="/usr/bin/doas"
elif [ "$(id -u)" -ne 0 ]; then
echo "Tailscale system service installation requires root privileges, but sudo/doas is not available." >&2
echo "Either install sudo, configure doas, or run home-manager as root." >&2
exit 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium

Using exit 1 here will abort the entire home-manager activation if sudo/doas is unavailable, potentially leaving the system in a partially configured state. Consider using a warning instead of a hard error, or add a configuration option to make system service installation optional. This is especially problematic since the module has installSystemService = true by default.

Agent: 🏛 Architecture • Fix in Cursor • Fix in Claude

Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#404
File: home-manager/modules/tailscale/default.nix#L236
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.

Feedback:
Using `exit 1` here will abort the entire home-manager activation if sudo/doas is unavailable, potentially leaving the system in a partially configured state. Consider using a warning instead of a hard error, or add a configuration option to make system service installation optional. This is especially problematic since the module has `installSystemService = true` by default.

fi

run_root_cmd() {
if [ -n "$SUDO_CMD" ]; then
''${DRY_RUN_CMD:-} "$SUDO_CMD" "$@"
else
''${DRY_RUN_CMD:-} "$@"
fi
}

# Only install if service file differs from nix-generated one
if ! cmp -s "$NIX_SERVICE" "$SERVICE_FILE" 2>/dev/null; then
echo "Installing tailscaled systemd service (requires root)..."
run_root_cmd cp "$NIX_SERVICE" "$SERVICE_FILE"
run_root_cmd systemctl daemon-reload
run_root_cmd systemctl enable tailscaled
echo "Tailscaled service installed."
fi

# Configure sudo to include Nix profile paths
echo "Configuring sudo PATH for Nix packages..."
SUDOERS_CONTENT="# Added by home-manager for Nix Tailscale

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Medium

Modifying /etc/sudoers.d/ during every home-manager activation creates a permanent system-level change that persists even after uninstalling the module. This violates the principle of declarative configuration management. Consider: 1) Documenting this as a manual setup step, 2) Adding a cleanup mechanism in a deactivation hook, or 3) Using a different approach that doesn't require persistent sudoers modifications.

Agent: 🏛 Architecture • Fix in Cursor • Fix in Claude

Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#404
File: home-manager/modules/tailscale/default.nix#L258
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.

Feedback:
Modifying `/etc/sudoers.d/` during every home-manager activation creates a permanent system-level change that persists even after uninstalling the module. This violates the principle of declarative configuration management. Consider: 1) Documenting this as a manual setup step, 2) Adding a cleanup mechanism in a deactivation hook, or 3) Using a different approach that doesn't require persistent sudoers modifications.

Defaults secure_path=\"${config.home.homeDirectory}/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin\"
"

# Create temporary file with correct content
TEMP_SUDOERS=$(mktemp)
echo "$SUDOERS_CONTENT" > "$TEMP_SUDOERS"

# Only update if different or doesn't exist
if ! cmp -s "$TEMP_SUDOERS" "$SUDOERS_FILE" 2>/dev/null; then
run_root_cmd cp "$TEMP_SUDOERS" "$SUDOERS_FILE"
run_root_cmd chmod 0440 "$SUDOERS_FILE"
echo "Sudo PATH configured. You can now use: sudo tailscale login"
fi

rm -f "$TEMP_SUDOERS"
Comment on lines +219 to +273

Copilot AI Dec 10, 2025

Copy link

Choose a reason for hiding this comment

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

[nitpick] The shell script has inconsistent and excessive indentation (using many leading spaces). This makes the code harder to read and maintain. Consider aligning the script content to the left margin with minimal indentation, similar to the style used in the createTailscaleDirs activation script (lines 202-213).

Suggested change
SERVICE_FILE="/etc/systemd/system/tailscaled.service"
NIX_SERVICE="${tailscaledServiceFile}"
SUDOERS_FILE="/etc/sudoers.d/nix-tailscale"
# Resolve an elevated command helper
SUDO_CMD=""
if command -v sudo >/dev/null 2>&1; then
SUDO_CMD="sudo"
elif [ -x /usr/bin/sudo ]; then
SUDO_CMD="/usr/bin/sudo"
elif command -v doas >/dev/null 2>&1; then
SUDO_CMD="doas"
elif [ -x /usr/bin/doas ]; then
SUDO_CMD="/usr/bin/doas"
elif [ "$(id -u)" -ne 0 ]; then
echo "Tailscale system service installation requires root privileges, but sudo/doas is not available." >&2
echo "Either install sudo, configure doas, or run home-manager as root." >&2
exit 1
fi
run_root_cmd() {
if [ -n "$SUDO_CMD" ]; then
''${DRY_RUN_CMD:-} "$SUDO_CMD" "$@"
else
''${DRY_RUN_CMD:-} "$@"
fi
}
# Only install if service file differs from nix-generated one
if ! cmp -s "$NIX_SERVICE" "$SERVICE_FILE" 2>/dev/null; then
echo "Installing tailscaled systemd service (requires root)..."
run_root_cmd cp "$NIX_SERVICE" "$SERVICE_FILE"
run_root_cmd systemctl daemon-reload
run_root_cmd systemctl enable tailscaled
echo "Tailscaled service installed."
fi
# Configure sudo to include Nix profile paths
echo "Configuring sudo PATH for Nix packages..."
SUDOERS_CONTENT="# Added by home-manager for Nix Tailscale
Defaults secure_path=\"${config.home.homeDirectory}/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin\"
"
# Create temporary file with correct content
TEMP_SUDOERS=$(mktemp)
echo "$SUDOERS_CONTENT" > "$TEMP_SUDOERS"
# Only update if different or doesn't exist
if ! cmp -s "$TEMP_SUDOERS" "$SUDOERS_FILE" 2>/dev/null; then
run_root_cmd cp "$TEMP_SUDOERS" "$SUDOERS_FILE"
run_root_cmd chmod 0440 "$SUDOERS_FILE"
echo "Sudo PATH configured. You can now use: sudo tailscale login"
fi
rm -f "$TEMP_SUDOERS"
SERVICE_FILE="/etc/systemd/system/tailscaled.service"
NIX_SERVICE="${tailscaledServiceFile}"
SUDOERS_FILE="/etc/sudoers.d/nix-tailscale"
# Resolve an elevated command helper
SUDO_CMD=""
if command -v sudo >/dev/null 2>&1; then
SUDO_CMD="sudo"
elif [ -x /usr/bin/sudo ]; then
SUDO_CMD="/usr/bin/sudo"
elif command -v doas >/dev/null 2>&1; then
SUDO_CMD="doas"
elif [ -x /usr/bin/doas ]; then
SUDO_CMD="/usr/bin/doas"
elif [ "$(id -u)" -ne 0 ]; then
echo "Tailscale system service installation requires root privileges, but sudo/doas is not available." >&2
echo "Either install sudo, configure doas, or run home-manager as root." >&2
exit 1
fi
run_root_cmd() {
if [ -n "$SUDO_CMD" ]; then
''${DRY_RUN_CMD:-} "$SUDO_CMD" "$@"
else
''${DRY_RUN_CMD:-} "$@"
fi
}
# Only install if service file differs from nix-generated one
if ! cmp -s "$NIX_SERVICE" "$SERVICE_FILE" 2>/dev/null; then
echo "Installing tailscaled systemd service (requires root)..."
run_root_cmd cp "$NIX_SERVICE" "$SERVICE_FILE"
run_root_cmd systemctl daemon-reload
run_root_cmd systemctl enable tailscaled
echo "Tailscaled service installed."
fi
# Configure sudo to include Nix profile paths
echo "Configuring sudo PATH for Nix packages..."
SUDOERS_CONTENT="# Added by home-manager for Nix Tailscale
Defaults secure_path=\"${config.home.homeDirectory}/.nix-profile/bin:/nix/var/nix/profiles/default/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin\"
"
# Create temporary file with correct content
TEMP_SUDOERS=$(mktemp)
echo "$SUDOERS_CONTENT" > "$TEMP_SUDOERS"
# Only update if different or doesn't exist
if ! cmp -s "$TEMP_SUDOERS" "$SUDOERS_FILE" 2>/dev/null; then
run_root_cmd cp "$TEMP_SUDOERS" "$SUDOERS_FILE"
run_root_cmd chmod 0440 "$SUDOERS_FILE"
echo "Sudo PATH configured. You can now use: sudo tailscale login"
fi
rm -f "$TEMP_SUDOERS"

Copilot uses AI. Check for mistakes.
''
Comment on lines +219 to +274

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.

security-high high

This activation script performs significant system-level modifications, including copying service files to /etc/systemd/system and modifying /etc/sudoers.d. While DRY_RUN_CMD is used, direct manipulation of /etc by a home-manager module is a powerful operation. It's crucial to ensure these operations are thoroughly tested and that the sudoers file modification is strictly necessary and correctly configured to prevent unintended privilege escalation or system misconfiguration. Consider if there's a more declarative NixOS-native way to manage systemd services and sudoers entries if this configuration is intended for NixOS systems, or if this is strictly for non-NixOS Linux where home-manager acts more like a script runner.

);
};
}
Loading
Loading