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
36 changes: 18 additions & 18 deletions flake.lock

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

15 changes: 13 additions & 2 deletions home-manager/services/cliproxyapi/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@ let
rsync = "${pkgs.rsync}/bin/rsync";
};

# Wrapper script that runs start.sh with docker group permissions
# Note: sg is from shadow package, available as system binary /usr/bin/sg
dockerStartScript = pkgs.writeShellScript "cliproxyapi-docker-start" ''

Copilot AI Dec 28, 2025

Copy link

Choose a reason for hiding this comment

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

The dockerStartScript wrapper unconditionally uses 'sg docker' to run the start script with docker group permissions, but this will fail if the user is not in the docker group. The systemd service should either ensure the user is in the docker group before starting, or the wrapper script should handle this case more gracefully with an appropriate error message directing the user to run docker-setup.

Suggested change
dockerStartScript = pkgs.writeShellScript "cliproxyapi-docker-start" ''
dockerStartScript = pkgs.writeShellScript "cliproxyapi-docker-start" ''
# Ensure sg exists
if ! command -v sg >/dev/null 2>&1; then
echo "ERROR: 'sg' command not found at runtime. Please ensure the 'shadow' tools are installed or run without docker group switching." >&2
exit 1
fi
# Ensure current user belongs to the docker group before attempting to switch
if ! id -nG "$USER" 2>/dev/null | grep -qw docker; then
cat >&2 <<'EOF'
ERROR: Current user is not in the 'docker' group, so the cliproxyapi docker wrapper cannot switch to docker group permissions.
Please run 'docker-setup' to configure Docker and add your user to the 'docker' group, then log out and log back in before retrying.
EOF
exit 1
fi

Copilot uses AI. Check for mistakes.
exec /usr/bin/sg docker -c "${pkgs.bash}/bin/bash ${startScript}"
Comment on lines +16 to +17

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Use Nix-provided sg path for docker group

The wrapper hard-codes /usr/bin/sg, which does not exist on NixOS (and other Nix-based systems that don’t populate /usr/bin). In those environments the user service will fail with ENOENT before it can run start.sh, so cliproxyapi never starts even though the rest of the Nix paths are valid. Consider using a Nix-provided sg (e.g., ${pkgs.shadow}/bin/sg) or otherwise ensuring the binary exists on the target system.

Useful? React with 👍 / 👎.

Comment on lines +15 to +17

Copilot AI Dec 28, 2025

Copy link

Choose a reason for hiding this comment

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

The dockerStartScript wrapper uses a hardcoded system path /usr/bin/sg instead of referencing the Nix package. This creates a dependency on the system's shadow package being available, which may not be present or could be incompatible across different Linux distributions. Consider using the Nix package path for sg or adding a check to verify its availability.

Suggested change
# Note: sg is from shadow package, available as system binary /usr/bin/sg
dockerStartScript = pkgs.writeShellScript "cliproxyapi-docker-start" ''
exec /usr/bin/sg docker -c "${pkgs.bash}/bin/bash ${startScript}"
# Note: sg is from shadow package
dockerStartScript = pkgs.writeShellScript "cliproxyapi-docker-start" ''
exec ${pkgs.shadow}/bin/sg docker -c "${pkgs.bash}/bin/bash ${startScript}"

Copilot uses AI. Check for mistakes.

@cubic-dev-ai cubic-dev-ai Bot Dec 28, 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: Hardcoded /usr/bin/sg path is not portable, especially on NixOS where system binaries are not in /usr/bin. Consider using ${pkgs.shadow}/bin/sg for consistent path resolution across environments.

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

<comment>Hardcoded `/usr/bin/sg` path is not portable, especially on NixOS where system binaries are not in `/usr/bin`. Consider using `${pkgs.shadow}/bin/sg` for consistent path resolution across environments.</comment>

<file context>
@@ -11,6 +11,12 @@ let
+  # Wrapper script that runs start.sh with docker group permissions
+  # Note: sg is from shadow package, available as system binary /usr/bin/sg
+  dockerStartScript = pkgs.writeShellScript &quot;cliproxyapi-docker-start&quot; &#39;&#39;
+    exec /usr/bin/sg docker -c &quot;${pkgs.bash}/bin/bash ${startScript}&quot;
+  &#39;&#39;;
+
</file context>
Fix with Cubic

'';
Comment on lines +14 to +18

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 | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, check if the file exists and read the relevant lines
if [ -f "home-manager/services/cliproxyapi/default.nix" ]; then
  echo "File found. Reading lines 1-25 for context:"
  cat -n "home-manager/services/cliproxyapi/default.nix" | head -25
else
  echo "File not found at specified path"
  find . -name "default.nix" -path "*/cliproxyapi*" 2>/dev/null
fi

Repository: shunkakinoki/dotfiles

Length of output: 1243


🏁 Script executed:

# Check if pkgs.shadow is used elsewhere in the Nix codebase
echo "=== Checking for shadow package usage in Nix files ==="
rg 'pkgs\.shadow' --type nix || echo "No shadow package references found"

echo -e "\n=== Checking for /usr/bin/sg usage ==="
rg '/usr/bin/sg' --type nix || echo "No hardcoded /usr/bin/sg found"

echo -e "\n=== Checking for sg usage patterns ==="
rg 'sg ' --type nix -A 2 | head -40

Repository: shunkakinoki/dotfiles

Length of output: 929


🏁 Script executed:

# Check the structure and conventions used in other home-manager services
echo "=== Listing home-manager/services structure ==="
if [ -d "home-manager/services" ]; then
  find home-manager/services -name "default.nix" -type f | head -10
else
  echo "home-manager/services directory not found"
fi

Repository: shunkakinoki/dotfiles

Length of output: 516


Replace hardcoded /usr/bin/sg with ${pkgs.shadow}/bin/sg for consistency and portability.

Line 17 uses a hardcoded absolute path to sg, which breaks the established pattern in this file (lines 9–11, 21–24) where all package binaries use ${pkgs.XXX}/bin/YYY. The comment acknowledges sg is from the shadow package but does not reference it; use ${pkgs.shadow}/bin/sg instead to ensure portability across NixOS and other Linux distributions.

Optional: Consider adding validation that the user is in the docker group, since sg will fail silently if group membership has not been activated after running docker-setup or following home-manager activation before re-login.

🤖 Prompt for AI Agents
In home-manager/services/cliproxyapi/default.nix around lines 14 to 18, the
wrapper script hardcodes /usr/bin/sg which breaks the file's established pattern
and can be non-portable; replace the hardcoded path with the package reference
${pkgs.shadow}/bin/sg in the dockerStartScript exec line so the script uses the
Nix-provided shadow binary, and optionally add a small runtime check (or
message) to validate that the current user is in the docker group before calling
sg to avoid silent failures after activation.


# Create backup scripts with paths substituted at build time
backupAuthScript = pkgs.replaceVars ./scripts/backup-auth.sh {
aws = "${pkgs.awscli2}/bin/aws";
Expand Down Expand Up @@ -50,7 +56,11 @@ in
systemd.user.services.cliproxyapi = lib.mkIf pkgs.stdenv.isLinux {
Unit = {
Description = "CLI Proxy API server";
After = [ "network.target" ];
After = [
"network.target"
"docker.service"
];
Wants = [ "docker.service" ];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

User service cannot depend on system docker service

The systemd.user.services.cliproxyapi service declares After and Wants dependencies on docker.service, but this won't work as intended. Systemd user services run in a separate instance from system services, so user units cannot properly depend on or reference system units like docker.service. The Wants directive will be unable to find the unit, and the After ordering will not function correctly. This means the cliproxyapi service may attempt to start before the Docker daemon is running, causing the docker run command in start.sh to fail.

Fix in Cursor Fix in Web

Copilot AI Dec 28, 2025

Copy link

Choose a reason for hiding this comment

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

The systemd service configuration uses 'Wants' for docker.service, which means the cliproxyapi service will start even if docker.service fails to start. Given that the Docker container is required for the service to function on Linux, this should be 'Requires' instead of 'Wants' to ensure the service fails if Docker is not available.

Suggested change
Wants = [ "docker.service" ];
Requires = [ "docker.service" ];

Copilot uses AI. Check for mistakes.
};
Service = {
Type = "simple";
Expand All @@ -60,9 +70,10 @@ in
pkgs.bash
pkgs.coreutils
pkgs.awscli2
pkgs.docker
]
}";
ExecStart = "${pkgs.bash}/bin/bash ${startScript}";
ExecStart = "${dockerStartScript}";
Restart = "always";
RestartSec = 3;
};
Expand Down
25 changes: 23 additions & 2 deletions home-manager/services/cliproxyapi/scripts/start.sh
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,33 @@ fi
# Change to config dir so logs are created there
cd "$CONFIG_DIR"

# Find and exec cliproxyapi with config file
# On Linux, prefer Docker for easy upgrades
if [ "$(uname)" = "Linux" ] && command -v docker >/dev/null 2>&1; then
# Stop any existing container
docker rm -f cliproxyapi 2>/dev/null || true
Comment on lines +100 to +101

Copilot AI Dec 28, 2025

Copy link

Choose a reason for hiding this comment

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

The error handling for Docker removal uses '|| true' which silently ignores all errors. While this is acceptable for the specific case where the container doesn't exist, it could also hide more serious errors like Docker daemon connectivity issues or permission problems. Consider adding specific error handling or at least logging warnings for unexpected failures.

Suggested change
# Stop any existing container
docker rm -f cliproxyapi 2>/dev/null || true
# Stop any existing container if it exists
if docker ps -a --format '{{.Names}}' | grep -qx cliproxyapi; then
docker rm -f cliproxyapi
fi

Copilot uses AI. Check for mistakes.

# Create logs directory if it doesn't exist
mkdir -p "$CONFIG_DIR/logs"

exec docker run --rm \
--name cliproxyapi \
--network host \
Comment on lines +106 to +108

Copilot AI Dec 28, 2025

Copy link

Choose a reason for hiding this comment

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

The Docker container is configured with host networking mode (--network host), which bypasses Docker's network isolation and exposes all container ports directly to the host. This could create security concerns as the container has full network access. Consider using explicit port mapping (-p) instead unless host networking is specifically required for functionality.

Suggested change
exec docker run --rm \
--name cliproxyapi \
--network host \
# Run container with explicit port mapping instead of host networking.
# CLIPROXYAPI_PORT can be set to change the exposed port; defaults to 8080.
exec docker run --rm \
--name cliproxyapi \
-p "${CLIPROXYAPI_PORT:-8080}:${CLIPROXYAPI_PORT:-8080}" \

Copilot uses AI. Check for mistakes.
--ulimit nofile=65536:65536 \

Copilot AI Dec 28, 2025

Copy link

Choose a reason for hiding this comment

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

The volume mount maps the host's $CONFIG_DIR to /root/.cli-proxy-api in the container. This assumes the container process runs as root, which is a security anti-pattern. If the container is compromised, the attacker has root privileges. Additionally, this creates files owned by root on the host filesystem, which could cause permission issues. Consider running the container with a non-root user using the --user flag.

Suggested change
--ulimit nofile=65536:65536 \
--ulimit nofile=65536:65536 \
--user "$(id -u):$(id -g)" \

Copilot uses AI. Check for mistakes.
-v "$CONFIG:/CLIProxyAPI/config.yaml:ro" \
-v "$CONFIG_DIR:/root/.cli-proxy-api" \
-v "$CONFIG_DIR/logs:/CLIProxyAPI/logs" \

Copilot AI Dec 28, 2025

Copy link

Choose a reason for hiding this comment

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

The Docker run command only passes the MANAGEMENT_PASSWORD environment variable to the container, but the start script sets up several other environment variables earlier (OBJECTSTORE_ENDPOINT, OBJECTSTORE_BUCKET, OBJECTSTORE_ACCESS_KEY, OBJECTSTORE_SECRET_KEY) that may be needed by the containerized application. If these are required for the application to function properly, they should also be passed to the container using additional -e flags.

Suggested change
-v "$CONFIG_DIR/logs:/CLIProxyAPI/logs" \
-v "$CONFIG_DIR/logs:/CLIProxyAPI/logs" \
-e OBJECTSTORE_ENDPOINT="${OBJECTSTORE_ENDPOINT:-}" \
-e OBJECTSTORE_BUCKET="${OBJECTSTORE_BUCKET:-}" \
-e OBJECTSTORE_ACCESS_KEY="${OBJECTSTORE_ACCESS_KEY:-}" \
-e OBJECTSTORE_SECRET_KEY="${OBJECTSTORE_SECRET_KEY:-}" \

Copilot uses AI. Check for mistakes.
-e MANAGEMENT_PASSWORD="${MANAGEMENT_PASSWORD:-}" \
eceasy/cli-proxy-api:latest

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

Using the :latest tag for the Docker image is convenient for development but can be risky for a service that runs continuously. When the eceasy/cli-proxy-api:latest image is updated, your service might pull a new version with breaking changes automatically, leading to unexpected downtime or behavior.

For better stability and reproducibility, it's a best practice to pin the image to a specific version tag (e.g., :v1.2.3) or, even better, to its immutable content digest (e.g., @sha256:...). This ensures that your service always runs the exact version you have tested and that upgrades are a deliberate action.

You can find the digest by running docker inspect eceasy/cli-proxy-api:latest after pulling the image.

Suggested change
eceasy/cli-proxy-api:latest
eceasy/cli-proxy-api:latest # TODO: Pin to a specific version or digest for stability

Copilot AI Dec 28, 2025

Copy link

Choose a reason for hiding this comment

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

The docker run invocation uses the external image eceasy/cli-proxy-api:latest pinned only to the mutable latest tag, which introduces a supply chain risk: whenever the local image is missing or refreshed, arbitrary code published under that tag will be pulled and executed with host networking and access to your config/log volumes. If an attacker compromises the upstream Docker Hub repository or its build pipeline, they could push a malicious latest image that this service would then run, leading to code execution and exfiltration of secrets such as MANAGEMENT_PASSWORD and API keys from the mounted config. To mitigate this, pin the container to an immutable digest or specific version tag and update that reference deliberately as part of a controlled upgrade process.

Copilot uses AI. Check for mistakes.
fi
Comment on lines +98 to +115

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 | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, verify the script exists and check the lines in question
cat -n home-manager/services/cliproxyapi/scripts/start.sh | sed -n '98,115p'

Repository: shunkakinoki/dotfiles

Length of output: 795


🏁 Script executed:

# Check if there are any version pinning mechanisms elsewhere
fd -e "yml" -e "yaml" -e "toml" -e "json" . home-manager/services/cliproxyapi/ | head -20

Repository: shunkakinoki/dotfiles

Length of output: 47


🏁 Script executed:

# Search for network requirement documentation in the codebase
rg -i "network.*host|cli.*proxy.*api.*network|docker.*network" home-manager/services/cliproxyapi/ -A 2 -B 2

Repository: shunkakinoki/dotfiles

Length of output: 471


🏁 Script executed:

# Check the full start.sh script for context about version management
wc -l home-manager/services/cliproxyapi/scripts/start.sh

Repository: shunkakinoki/dotfiles

Length of output: 119


🌐 Web query:

eceasy/cli-proxy-api docker image default user runtime

💡 Result:

The eceasy/cli-proxy-api image runs as root by default (paths in the docs use /root for runtime data). [1][2]

Sources:
[1] eceasy on Docker Hub (cli-proxy-api).
[2] CLIProxyAPI "Run with Docker" instructions showing volumes under /root/.cli-proxy-api.


Pin the Docker image version instead of using :latest tag.

Using eceasy/cli-proxy-api:latest is risky for production as it can pull different versions on different machines and makes rollbacks difficult. Pin to a specific version tag.

🔎 Suggested fix
-    eceasy/cli-proxy-api:latest
+    eceasy/cli-proxy-api:v1.2.3

The --network host flag is appropriate for proxy functionality.

Running with --network host is a necessary design choice for proxy services to bind to host network interfaces. This is standard for proxy containers and the trade-off is intentional.


Consider validating Docker daemon availability before executing.

The script checks for the Docker binary but doesn't verify the daemon is running. If the daemon is down, docker run will fail with a cryptic error.

🔎 Optional validation check
 # On Linux, prefer Docker for easy upgrades
 if [ "$(uname)" = "Linux" ] && command -v docker >/dev/null 2>&1; then
+  # Verify Docker daemon is accessible
+  if ! docker info >/dev/null 2>&1; then
+    echo "Docker daemon is not running." >&2
+    exit 1
+  fi
+
   # Stop any existing container
   docker rm -f cliproxyapi 2>/dev/null || true
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# On Linux, prefer Docker for easy upgrades
if [ "$(uname)" = "Linux" ] && command -v docker >/dev/null 2>&1; then
# Stop any existing container
docker rm -f cliproxyapi 2>/dev/null || true
# Create logs directory if it doesn't exist
mkdir -p "$CONFIG_DIR/logs"
exec docker run --rm \
--name cliproxyapi \
--network host \
--ulimit nofile=65536:65536 \
-v "$CONFIG:/CLIProxyAPI/config.yaml:ro" \
-v "$CONFIG_DIR:/root/.cli-proxy-api" \
-v "$CONFIG_DIR/logs:/CLIProxyAPI/logs" \
-e MANAGEMENT_PASSWORD="${MANAGEMENT_PASSWORD:-}" \
eceasy/cli-proxy-api:latest
fi
# On Linux, prefer Docker for easy upgrades
if [ "$(uname)" = "Linux" ] && command -v docker >/dev/null 2>&1; then
# Verify Docker daemon is accessible
if ! docker info >/dev/null 2>&1; then
echo "Docker daemon is not running." >&2
exit 1
fi
# Stop any existing container
docker rm -f cliproxyapi 2>/dev/null || true
# Create logs directory if it doesn't exist
mkdir -p "$CONFIG_DIR/logs"
exec docker run --rm \
--name cliproxyapi \
--network host \
--ulimit nofile=65536:65536 \
-v "$CONFIG:/CLIProxyAPI/config.yaml:ro" \
-v "$CONFIG_DIR:/root/.cli-proxy-api" \
-v "$CONFIG_DIR/logs:/CLIProxyAPI/logs" \
-e MANAGEMENT_PASSWORD="${MANAGEMENT_PASSWORD:-}" \
eceasy/cli-proxy-api:v1.2.3
fi


# macOS: use Homebrew binary
if [ -x /opt/homebrew/bin/cliproxyapi ]; then
exec /opt/homebrew/bin/cliproxyapi -config "$CONFIG" "$@"
elif [ -x /usr/local/bin/cliproxyapi ]; then
exec /usr/local/bin/cliproxyapi -config "$CONFIG" "$@"
else
echo 'cliproxyapi binary not found; install it with "brew install cliproxyapi"' >&2
echo 'cliproxyapi not found' >&2
echo 'Linux: Docker should be available' >&2
echo 'macOS: brew install cliproxyapi' >&2
exit 1
fi
2 changes: 2 additions & 0 deletions home-manager/services/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ let
brewUpgrader = import ./brew-upgrader { inherit pkgs; };
cliproxyapi = import ./cliproxyapi { inherit pkgs; };
codeSyncer = import ./code-syncer { inherit pkgs; };
docker = import ./docker { inherit lib pkgs; };
dotfilesUpdater = import ./dotfiles-updater { inherit pkgs; };
neversslKeepalive = import ./neverssl-keepalive { inherit pkgs; };
ollama = import ./ollama { inherit pkgs; };
Expand All @@ -19,6 +20,7 @@ in
brewUpgrader
cliproxyapi
codeSyncer
docker
dotfilesUpdater
neversslKeepalive
ollama
Expand Down
64 changes: 64 additions & 0 deletions home-manager/services/docker/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
{ pkgs, lib, ... }:
let
# Systemd service file for Docker daemon
dockerServiceFile = pkgs.writeText "docker.service" ''
[Unit]
Description=Docker Application Container Engine
Documentation=https://docs.docker.com
After=network-online.target
Wants=network-online.target

[Service]
Type=notify
ExecStart=${pkgs.docker}/bin/dockerd
ExecReload=${pkgs.coreutils}/bin/kill -s HUP $MAINPID
Restart=always
RestartSec=10s

[Install]
WantedBy=multi-user.target
'';

# Script to ensure user is in docker group and system docker is running
setupDockerScript = pkgs.writeShellScript "setup-docker" ''

Copilot AI Dec 28, 2025

Copy link

Choose a reason for hiding this comment

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

The variable name 'setupDockerScript' is inconsistent with the actual command name 'docker-setup'. For better code clarity and maintainability, the variable should be named consistently with the exposed command, such as 'dockerSetupScript'.

Copilot uses AI. Check for mistakes.
set -euo pipefail

# Define paths
GROUPS=${pkgs.shadow}/bin/groups
GREP=${pkgs.gnugrep}/bin/grep
USERMOD=${pkgs.shadow}/bin/usermod
SYSTEMCTL=${pkgs.systemd}/bin/systemctl
TEE=${pkgs.coreutils}/bin/tee
DOCKER_SERVICE_FILE=${dockerServiceFile}

# Check if docker group exists and user is in it
if ! $GROUPS | $GREP -q docker; then
echo "Adding user to docker group..."
sudo $USERMOD -aG docker $USER
echo "✅ Added to docker group. Please log out and back in, or run: newgrp docker"
Comment on lines +34 to +38

Copilot AI Dec 28, 2025

Copy link

Choose a reason for hiding this comment

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

The script unconditionally attempts to add the user to the docker group without first checking if the docker group exists on the system. If the docker group doesn't exist (e.g., Docker not installed or not installed via package manager), the usermod command will fail. Add a check to verify the docker group exists before attempting to add the user to it.

Suggested change
# Check if docker group exists and user is in it
if ! groups | grep -q docker; then
echo "Adding user to docker group..."
sudo usermod -aG docker $USER
echo "✅ Added to docker group. Please log out and back in, or run: newgrp docker"
# Ensure docker group exists, then check if user is in it
if getent group docker >/dev/null 2>&1; then
if ! groups | grep -q docker; then
echo "Adding user to docker group..."
sudo usermod -aG docker "$USER"
echo "✅ Added to docker group. Please log out and back in, or run: newgrp docker"
fi
else
echo "⚠️ 'docker' group does not exist. Please install/configure Docker so that the group is created, then re-run this script."

Copilot uses AI. Check for mistakes.
fi

# Check if system docker service exists and is running
if ! $SYSTEMCTL is-active --quiet docker 2>/dev/null; then
echo "Starting Docker daemon..."
if [ ! -f /etc/systemd/system/docker.service ]; then
echo "Installing Docker systemd service..."
sudo $TEE /etc/systemd/system/docker.service > /dev/null < "$DOCKER_SERVICE_FILE"
sudo $SYSTEMCTL daemon-reload
sudo $SYSTEMCTL enable docker
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nix store path in system service becomes stale

The setup script writes ${pkgs.docker}/bin/dockerd (a Nix store path) into /etc/systemd/system/docker.service, but this file is only created once (due to the [ ! -f ... ] check on line 19). When home-manager updates and pkgs.docker changes to a new store path, the service file retains the old path. After Nix garbage collection runs, the old store path gets deleted, causing the system docker service to fail with "file not found". The system service file becomes permanently broken until manually updated.

Fix in Cursor Fix in Web

Comment thread
coderabbitai[bot] marked this conversation as resolved.
sudo $SYSTEMCTL start docker
echo "✅ Docker daemon started"
else
echo "✅ Docker daemon is already running"
fi
Comment on lines +41 to +54

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

This script attempts to install and enable a system-wide docker.service file if it doesn't exist. Managing system-level services imperatively from a user's Home Manager configuration is highly discouraged and can lead to an unstable or broken system. It goes against the declarative nature of Nix and can conflict with the system's package manager.

Docker should be installed and managed at the system level (e.g., via configuration.nix on NixOS with virtualisation.docker.enable = true;, or the system's package manager on other distributions).

I suggest modifying this script to only attempt to start the Docker service if it's not running, and provide a helpful error message if it fails, rather than trying to install the service file itself.

    # Check if system docker service is running
    if systemctl is-active --quiet docker 2>/dev/null; then
      echo "✅ Docker daemon is already running"
    else
      echo "Docker daemon is not running. Attempting to start it..."
      if sudo systemctl start docker 2>/dev/null; then
        echo "✅ Docker daemon started"
      else
        echo "❌ Failed to start Docker daemon." >&2
        echo "   Please ensure Docker is installed and enabled on your system." >&2
        echo "   On NixOS, add 'virtualisation.docker.enable = true;' to your configuration.nix." >&2
        echo "   On other systems, use the appropriate package manager." >&2
        exit 1
      fi
    fi

'';
in
{
# Provide setup script for system Docker
home.packages = lib.mkIf pkgs.stdenv.isLinux [
(pkgs.writeShellScriptBin "docker-setup" ''
exec ${setupDockerScript}
'')
];
}
4 changes: 2 additions & 2 deletions spec/cliproxyapi_spec.sh
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,8 @@ The output should include '/usr/local/bin/cliproxyapi'
End

It 'shows error message when binary not found'
When run bash -c "grep 'cliproxyapi binary not found' '$SCRIPT'"
The output should include 'cliproxyapi binary not found'
When run bash -c "grep 'cliproxyapi not found' '$SCRIPT'"
The output should include 'cliproxyapi not found'
End

It 'suggests installation command in error message'
Expand Down
Loading