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
14 changes: 7 additions & 7 deletions config/cliproxyapi/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,9 @@ remote-management:
disable-control-panel: false
# Authentication directory (supports ~ for home directory). If you use Windows, please set the directory like this: `C:/cli-proxy-api/`
auth-dir: "~/.cli-proxy-api"
# API keys for authentication
# API keys for client authentication (optional - leave commented for open access)
# api-keys:
# - "your-api-key-1"
# - "your-api-key-2"
# - "your-api-key"

# Enable debug logging
debug: true
Expand All @@ -34,12 +33,11 @@ quota-exceeded:
switch-preview-model: true # Whether to automatically switch to a preview model when a quota is exceeded
# When true, enable authentication for the WebSocket API (/v1/ws).
ws-auth: false
# AMP
# AMP integration
ampcode:
upstream-url: "https://ampcode.com"
restrict-management-to-localhost: true
# amp-upstream-api-key: "" # Optional - use AMP_API_KEY env var or ~/.local/share/amp/secrets.json

upstream-api-key: "__AMP_UPSTREAM_API_KEY__"
restrict-management-to-localhost: false

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 restrict-management-to-localhost to false allows management access from any remote machine, which significantly increases the attack surface of the service. While access is still protected by a secret key, it's recommended to keep management endpoints restricted to localhost unless remote management is a strict requirement. If remote access is needed, consider firewalling the management port to a trusted set of IP addresses.

@cubic-dev-ai cubic-dev-ai Bot Dec 25, 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: Changing restrict-management-to-localhost from true to false relaxes a security restriction, allowing remote access to AMP management endpoints. Ensure this is intentional and that proper authentication/authorization is in place to protect these endpoints from unauthorized remote access.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At config/cliproxyapi/config.yaml, line 40:

<comment>Changing `restrict-management-to-localhost` from `true` to `false` relaxes a security restriction, allowing remote access to AMP management endpoints. Ensure this is intentional and that proper authentication/authorization is in place to protect these endpoints from unauthorized remote access.</comment>

<file context>
@@ -34,11 +33,11 @@ quota-exceeded:
-  restrict-management-to-localhost: true
-# amp-upstream-api-key: &quot;&quot; # Optional - use AMP_API_KEY env var or ~/.local/share/amp/secrets.json
+  upstream-api-key: &quot;__AMP_UPSTREAM_API_KEY__&quot;
+  restrict-management-to-localhost: false
 
 # Gemini API keys (preferred)
</file context>
Fix with Cubic

Copilot AI Dec 25, 2025

Copy link

Choose a reason for hiding this comment

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

The restrict-management-to-localhost setting is being changed from true to false. This opens up management endpoints to non-localhost access which could be a security risk if the API is exposed to untrusted networks. Ensure this change is intentional and that the management secret key is strong enough to prevent unauthorized access from remote locations.

Suggested change
restrict-management-to-localhost: false
restrict-management-to-localhost: true

Copilot uses AI. Check for mistakes.
# Gemini API keys (preferred)
# gemini-api-key:
# - api-key: "AIzaSy...01"
Expand Down Expand Up @@ -77,12 +75,14 @@ openai-compatibility:
- "__OPENROUTER_API_KEY__"
models:
- name: "z-ai/glm-4.6"
- name: "z-ai/glm-4.7"
- name: "z-ai"
base-url: "https://api.z.ai/api/coding/paas/v4"
api-keys:
- "__ZAI_API_KEY__"
models:
- name: "glm-4.6"
- name: "glm-4.7"

# payload: # Optional payload configuration
# default: # Default rules only set parameters when they are missing in the payload.
Expand Down
3 changes: 3 additions & 0 deletions config/opencode/opencode.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,9 @@
"models": {
"glm-4.6": {
"name": "GLM-4.6 (via Z-AI)",

Copilot AI Dec 25, 2025

Copy link

Choose a reason for hiding this comment

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

There is a trailing comma after the "name" field which violates JSON syntax. While JSONC (JSON with Comments) may be more lenient in some parsers, trailing commas after the last property in an object are not universally supported and could cause parsing errors.

Copilot uses AI. Check for mistakes.
},
"zai-coding-plan/glm-4.7": {
"name": "GLM-4.7 Coding Plan (via Z-AI)"
}
},
"options": {
Expand Down
13 changes: 11 additions & 2 deletions home-manager/services/cliproxyapi/default.nix
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
{ pkgs, ... }:
let
inherit (pkgs) lib;

# Create start script with paths substituted at build time
startScript = pkgs.replaceVars ./scripts/start.sh {
aws = "${pkgs.awscli2}/bin/aws";
sed = "${pkgs.gnused}/bin/sed";
};
in
{
# Main cliproxyapi service
Expand All @@ -9,14 +15,15 @@ in
config = {
ProgramArguments = [
"${pkgs.bash}/bin/bash"
"${./scripts/start.sh}"
"${startScript}"
];
Environment = {
HOME = "/Users/shunkakinoki";
PATH = "${
lib.makeBinPath [
pkgs.gnused
pkgs.coreutils
pkgs.awscli2
]
}:/opt/homebrew/bin:/usr/local/bin:/usr/bin";
};
Expand All @@ -38,9 +45,11 @@ in
lib.makeBinPath [
pkgs.gnused
pkgs.bash
pkgs.coreutils
pkgs.awscli2
]
}";
ExecStart = "${pkgs.bash}/bin/bash ${./scripts/start.sh}";
ExecStart = "${pkgs.bash}/bin/bash ${startScript}";
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 @@ -5,7 +5,8 @@ set -euo pipefail
CONFIG_DIR="$HOME/.cli-proxy-api"
TEMPLATE="$CONFIG_DIR/config.template.yaml"
CONFIG="$CONFIG_DIR/config.yaml"
ENV_FILE="$HOME/dotfiles/.env"
# Use explicit path since $HOME may not be set correctly in launchd context
ENV_FILE="${HOME:-/Users/shunkakinoki}/dotfiles/.env"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The hardcoded fallback path /Users/shunkakinoki makes the script non-portable and specific to a single user's machine setup. This harms reusability and maintainability. A better approach would be to pass the home directory path from the Nix configuration (e.g., using config.home.homeDirectory in default.nix) and substitute it into this script, avoiding any hardcoded user-specific paths.

Copilot AI Dec 25, 2025

Copy link

Choose a reason for hiding this comment

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

The HOME variable fallback uses a hardcoded username. While this may work for the specific user environment, consider making this more maintainable by using a configurable value passed from the Nix configuration where HOME is already set in the launchd Environment (line 21 of default.nix). The script could validate that HOME is set before using it, and fail early if it's not rather than falling back to a hardcoded path.

Copilot uses AI. Check for mistakes.
Comment on lines +8 to +9

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

Hardcoded user path breaks portability.

The fallback path /Users/shunkakinoki will fail for other users or on Linux systems. Consider using a more portable approach or failing explicitly when HOME is not set.

🔎 Suggested refactor for portable path handling
-# Use explicit path since $HOME may not be set correctly in launchd context
-ENV_FILE="${HOME:-/Users/shunkakinoki}/dotfiles/.env"
+# Use explicit path since $HOME may not be set correctly in launchd context
+# Fail explicitly if HOME is not set rather than using a hardcoded path
+if [ -z "${HOME:-}" ]; then
+  echo "ERROR: HOME environment variable not set" >&2
+  exit 1
+fi
+ENV_FILE="$HOME/dotfiles/.env"
📝 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
# Use explicit path since $HOME may not be set correctly in launchd context
ENV_FILE="${HOME:-/Users/shunkakinoki}/dotfiles/.env"
# Use explicit path since $HOME may not be set correctly in launchd context
# Fail explicitly if HOME is not set rather than using a hardcoded path
if [ -z "${HOME:-}" ]; then
echo "ERROR: HOME environment variable not set" >&2
exit 1
fi
ENV_FILE="$HOME/dotfiles/.env"
🤖 Prompt for AI Agents
In home-manager/services/cliproxyapi/scripts/start.sh around lines 8-9, remove
the hardcoded fallback /Users/shunkakinoki and instead require a valid HOME:
check if $HOME is set and non-empty and exit with a clear error if it isn't,
then construct ENV_FILE using "$HOME/dotfiles/.env" (no hardcoded user path) so
the script is portable across users and platforms.


# Source .env file to get API keys
if [ -f "$ENV_FILE" ]; then
Expand All @@ -26,14 +27,34 @@ export OBJECTSTORE_SECRET_KEY="${OBJECTSTORE_SECRET_KEY:-${AWS_SECRET_ACCESS_KEY

# Generate config from template with secrets injected
if [ -f "$TEMPLATE" ]; then
sed \
@sed@ \
-e "s|__OPENROUTER_API_KEY__|${OPENROUTER_API_KEY:-}|g" \
-e "s|__CLIPROXY_MANAGEMENT_PASSWORD__|${CLIPROXY_MANAGEMENT_PASSWORD:-}|g" \
-e "s|__ZAI_API_KEY__|${ZAI_API_KEY:-}|g" \
-e "s|__AMP_UPSTREAM_API_KEY__|${AMP_UPSTREAM_API_KEY:-}|g" \
"$TEMPLATE" >"$CONFIG"
# Also copy to objectstore config location (cliproxyapi uses this for persistence)
mkdir -p "$CONFIG_DIR/objectstore/config"
cp "$CONFIG" "$CONFIG_DIR/objectstore/config/config.yaml"

# Upload config to S3 to ensure backup is always correct
# This prevents corrupted configs from persisting across restarts
if [ -n "${OBJECTSTORE_ENDPOINT:-}" ] && [ -n "${OBJECTSTORE_ACCESS_KEY:-}" ]; then

Copilot AI Dec 25, 2025

Copy link

Choose a reason for hiding this comment

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

The S3 backup check only validates OBJECTSTORE_ENDPOINT and OBJECTSTORE_ACCESS_KEY, but the AWS CLI also requires OBJECTSTORE_SECRET_KEY to function properly. If the secret key is missing, the aws s3 cp command will fail. Add a check for OBJECTSTORE_SECRET_KEY in the condition to prevent attempting uploads that are guaranteed to fail.

Suggested change
if [ -n "${OBJECTSTORE_ENDPOINT:-}" ] && [ -n "${OBJECTSTORE_ACCESS_KEY:-}" ]; then
if [ -n "${OBJECTSTORE_ENDPOINT:-}" ] && [ -n "${OBJECTSTORE_ACCESS_KEY:-}" ] && [ -n "${OBJECTSTORE_SECRET_KEY:-}" ]; then

Copilot uses AI. Check for mistakes.
echo "Uploading config to S3 backup..." >&2
if AWS_ACCESS_KEY_ID="${OBJECTSTORE_ACCESS_KEY}" \

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 aws s3 cp command could potentially hang, blocking the service from starting. To improve reliability, consider wrapping the command with timeout to prevent it from running indefinitely. The timeout utility is available since pkgs.coreutils is in the PATH.

Suggested change
if AWS_ACCESS_KEY_ID="${OBJECTSTORE_ACCESS_KEY}" \
if timeout 30s AWS_ACCESS_KEY_ID="${OBJECTSTORE_ACCESS_KEY}" \

AWS_SECRET_ACCESS_KEY="${OBJECTSTORE_SECRET_KEY}" \
@aws@ s3 cp \
--endpoint-url="${OBJECTSTORE_ENDPOINT}" \
--no-progress \
"$CONFIG" \
"s3://cliproxyapi/config/config.yaml" 2>&1; then
echo "✅ Config backup uploaded" >&2
else
echo "⚠️ Config backup failed (continuing anyway)" >&2
fi
else
echo "⚠️ S3 config backup skipped: missing credentials" >&2
fi
Comment on lines +40 to +57

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

S3 backup logic has performance and configurability concerns.

Two issues to consider:

  1. Synchronous backup delays startup: The S3 upload happens synchronously during service initialization, which can significantly delay startup if there are network issues.

  2. Hardcoded bucket name: Line 51 uses a hardcoded bucket name "cliproxyapi" instead of using the ${OBJECTSTORE_BUCKET} variable that's already exported on line 24.

🔎 Suggested improvements

For the hardcoded bucket name, use the environment variable:

-      "s3://cliproxyapi/config/config.yaml" 2>&1; then
+      "s3://${OBJECTSTORE_BUCKET}/config/config.yaml" 2>&1; then

For the startup delay concern, consider:

  • Making the S3 backup asynchronous (backgrounded)
  • Or moving it to the separate backup service that already runs periodically
  • Or adding a timeout to the AWS CLI command
📝 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
# Upload config to S3 to ensure backup is always correct
# This prevents corrupted configs from persisting across restarts
if [ -n "${OBJECTSTORE_ENDPOINT:-}" ] && [ -n "${OBJECTSTORE_ACCESS_KEY:-}" ]; then
echo "Uploading config to S3 backup..." >&2
if AWS_ACCESS_KEY_ID="${OBJECTSTORE_ACCESS_KEY}" \
AWS_SECRET_ACCESS_KEY="${OBJECTSTORE_SECRET_KEY}" \
@aws@ s3 cp \
--endpoint-url="${OBJECTSTORE_ENDPOINT}" \
--no-progress \
"$CONFIG" \
"s3://cliproxyapi/config/config.yaml" 2>&1; then
echo "✅ Config backup uploaded" >&2
else
echo "⚠️ Config backup failed (continuing anyway)" >&2
fi
else
echo "⚠️ S3 config backup skipped: missing credentials" >&2
fi
# Upload config to S3 to ensure backup is always correct
# This prevents corrupted configs from persisting across restarts
if [ -n "${OBJECTSTORE_ENDPOINT:-}" ] && [ -n "${OBJECTSTORE_ACCESS_KEY:-}" ]; then
echo "Uploading config to S3 backup..." >&2
if AWS_ACCESS_KEY_ID="${OBJECTSTORE_ACCESS_KEY}" \
AWS_SECRET_ACCESS_KEY="${OBJECTSTORE_SECRET_KEY}" \
@aws@ s3 cp \
--endpoint-url="${OBJECTSTORE_ENDPOINT}" \
--no-progress \
"$CONFIG" \
"s3://${OBJECTSTORE_BUCKET}/config/config.yaml" 2>&1; then
echo "✅ Config backup uploaded" >&2
else
echo "⚠️ Config backup failed (continuing anyway)" >&2
fi
else
echo "⚠️ S3 config backup skipped: missing credentials" >&2
fi

fi

# Change to config dir so logs are created there
Expand Down
Loading