Skip to content

Add cpaplus support#5

Open
abwuge wants to merge 4 commits intobrokechubb:masterfrom
abwuge:master
Open

Add cpaplus support#5
abwuge wants to merge 4 commits intobrokechubb:masterfrom
abwuge:master

Conversation

@abwuge
Copy link
Copy Markdown

@abwuge abwuge commented Mar 21, 2026

No description provided.

Copilot AI review requested due to automatic review settings March 21, 2026 06:11
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a dedicated Linux installer for CLIProxyAPIPlus alongside the existing CLIProxyAPI installer, and updates repository documentation to cover both installers.

Changes:

  • Added cliproxyapiplus-installer with install/upgrade/status/auth/config tooling and systemd user-service management.
  • Updated README.md to document CLIProxyAPIPlus installation, usage, and service management alongside CLIProxyAPI.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 12 comments.

File Description
cliproxyapiplus-installer New Linux installer script for CLIProxyAPIPlus (download latest release, manage config, systemd user unit, uninstall, helpers).
README.md Expanded docs to include CLIProxyAPIPlus installer instructions, commands, directory layout, and service guidance.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +985 to +986
echo -e "${BLUE}Current API keys in config.yaml:${NC}"
grep -A 10 "^api-keys:" "${INSTALL_DIR}/config.yaml" | grep -v "^#" | head -10
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

check-config prints API keys from config.yaml in plaintext. This can leak secrets if the output is captured (support paste, logs). Consider removing this output or masking values by default (with an explicit --show-secrets style option if needed).

Suggested change
echo -e "${BLUE}Current API keys in config.yaml:${NC}"
grep -A 10 "^api-keys:" "${INSTALL_DIR}/config.yaml" | grep -v "^#" | head -10
echo -e "${BLUE}API keys are configured in config.yaml.${NC}"
echo -e "${YELLOW}For security reasons, key values are not displayed.${NC}"

Copilot uses AI. Check for mistakes.
Comment on lines +215 to +216
# Check if api-keys section exists and has non-empty values that look like real API keys
if grep -A 10 "^api-keys:" "$config_file" | grep -v "^#" | grep -v "^api-keys:" | grep -q '"sk-[^"]*"'; then
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

check_api_keys only treats keys matching "sk-..." as valid, but the installer guidance explicitly suggests non-sk- custom keys too. This will incorrectly report "NOT CONFIGURED" for valid custom keys. Either relax validation to accept any non-placeholder non-empty string(s) under api-keys:, or update the guidance to match the required format.

Suggested change
# Check if api-keys section exists and has non-empty values that look like real API keys
if grep -A 10 "^api-keys:" "$config_file" | grep -v "^#" | grep -v "^api-keys:" | grep -q '"sk-[^"]*"'; then
# Check if api-keys section exists and has at least one non-empty key entry
if grep -A 10 "^api-keys:" "$config_file" | grep -v "^#" | grep -v "^api-keys:" | grep -E -q '^[[:space:]]*-[[:space:]]*"[^"]+"'; then

Copilot uses AI. Check for mistakes.
local version_dir="$1"

local binary
binary=$(find "$version_dir" -name "CLIProxyAPIPlus" -o -name "cli-proxy-api-plus" -type f | head -1)
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

find "$version_dir" -name "CLIProxyAPIPlus" -o -name "cli-proxy-api-plus" -type f has an operator precedence issue: -type f only applies to the second -name, so it may return non-files for the first branch. Wrap the name predicates in parentheses so -type f applies to both.

Suggested change
binary=$(find "$version_dir" -name "CLIProxyAPIPlus" -o -name "cli-proxy-api-plus" -type f | head -1)
binary=$(find "$version_dir" \( -name "CLIProxyAPIPlus" -o -name "cli-proxy-api-plus" \) -type f | head -1)

Copilot uses AI. Check for mistakes.
Comment on lines +946 to +952
local temp_files
temp_files=$(find /tmp -name "tmp.*" -user "$(whoami)" 2>/dev/null || true)
if [[ -n "$temp_files" ]]; then
echo "$temp_files" | xargs rm -f 2>/dev/null || true
fi
}

Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

The cleanup trap deletes every /tmp/tmp.* file owned by the current user. That can remove unrelated temp files from other processes/sessions. Instead, track only the specific temp file(s) created by this script (e.g., the mktemp path) and delete just those.

Suggested change
local temp_files
temp_files=$(find /tmp -name "tmp.*" -user "$(whoami)" 2>/dev/null || true)
if [[ -n "$temp_files" ]]; then
echo "$temp_files" | xargs rm -f 2>/dev/null || true
fi
}
# Intentionally left blank to avoid deleting unrelated temporary files.
:
}

Copilot uses AI. Check for mistakes.
Comment on lines +937 to +942
# Remove installation directory
log_info "Removing CLIProxyAPIPlus installation..."
rm -rf "$INSTALL_DIR"

log_success "CLIProxyAPIPlus has been uninstalled successfully"
}
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

uninstall_cliproxyapiplus removes the install directory but leaves the user systemd unit behind (~/.config/systemd/user/cliproxyapiplus.service) and may leave the service enabled/failed. Consider stopping+disabling the service and removing the unit file (followed by systemctl --user daemon-reload / reset-failed) during uninstall.

Copilot uses AI. Check for mistakes.
Comment on lines +644 to +645
log_success "Created config.yaml from example with generated API keys"
log_info "Generated API keys: $generated_key1, $generated_key2"
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

This logs freshly generated API keys to stdout (Generated API keys: ...). That can leak secrets into terminal scrollback, CI logs, or shell history. Avoid printing full keys; at most print a masked version or only the config file path.

Suggested change
log_success "Created config.yaml from example with generated API keys"
log_info "Generated API keys: $generated_key1, $generated_key2"
# Do not log full API keys to avoid leaking secrets. Log only masked values.
local mask1="${generated_key1:0:4}****${generated_key1: -4}"
local mask2="${generated_key2:0:4}****${generated_key2: -4}"
log_success "Created config.yaml from example with generated API keys"
log_info "Generated API keys (masked): $mask1, $mask2"

Copilot uses AI. Check for mistakes.
Comment on lines +280 to +281
echo -e "${BLUE}Your API keys:${NC}"
grep -A 10 "^api-keys:" "${install_dir}/config.yaml" | grep -v "^#" | head -10
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

show_quick_start prints the configured API keys in plaintext. This is a secret disclosure risk (screen recording, shared terminals, logs). Prefer showing only whether keys are configured, or mask them (e.g., first/last 4 chars).

Suggested change
echo -e "${BLUE}Your API keys:${NC}"
grep -A 10 "^api-keys:" "${install_dir}/config.yaml" | grep -v "^#" | head -10
echo -e "${BLUE}Your API keys (status only, values hidden):${NC}"
grep -A 10 "^api-keys:" "${install_dir}/config.yaml" | grep -v "^#" | head -10 | awk -F':' '
NF >= 1 {
# Trim leading/trailing whitespace from key
key=$1
sub(/^[[:space:]]+/, "", key)
sub(/[[:space:]]+$/, "", key)
if (key == "api-keys" || key == "") {
print $0
} else {
# Determine if a non-empty value appears after the colon
value=$0
sub(/^[^:]*:/, "", value)
sub(/^[[:space:]]+/, "", value)
if (value != "" && value !~ /^#/) {
print key ": ******** (configured)"
} else {
print key ": (not configured)"
}
}
}
'

Copilot uses AI. Check for mistakes.
# Copy executable to main directory
if [[ -f "$executable" ]]; then
cp "$executable" "${INSTALL_DIR}/cli-proxy-api-plus"
log_success "Copied executable to ${INSTALL_DIR}/cli-proxy-api-plus"
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

setup_config only copies ${version_dir}/cli-proxy-api-plus into the install dir. If the release asset contains a differently named binary (you already look for CLIProxyAPIPlus in make_executable), the copy will be skipped and the later chmod +x ${INSTALL_DIR}/cli-proxy-api-plus will fail. Consider locating the extracted binary via the same find logic and failing with a clear error if it can’t be found/copied.

Suggested change
log_success "Copied executable to ${INSTALL_DIR}/cli-proxy-api-plus"
log_success "Copied executable to ${INSTALL_DIR}/cli-proxy-api-plus"
else
# Fallback: locate the executable using flexible naming (aligns with make_executable logic)
local found_exec
found_exec=$(find "$version_dir" -maxdepth 2 -type f \( -name 'cli-proxy-api-plus' -o -name 'CLIProxyAPIPlus' \) 2>/dev/null | head -n 1 || true)
if [[ -n "${found_exec:-}" ]]; then
cp "$found_exec" "${INSTALL_DIR}/cli-proxy-api-plus"
log_success "Copied executable from $(basename "$found_exec") to ${INSTALL_DIR}/cli-proxy-api-plus"
else
log_error "Unable to locate CLIProxyAPIPlus executable in ${version_dir}. Aborting installation."
exit 1
fi

Copilot uses AI. Check for mistakes.
Comment on lines +660 to +662
curl -L -o "$output" "$url"
else
wget -O "$output" "$url"
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

curl -L -o ... doesn’t fail the script on HTTP errors (e.g., 404/403) and can download an HTML error page that still passes the -f "$output" check. Use curl’s fail options (and consider retries/timeouts) so failed downloads are detected early with a clear error.

Suggested change
curl -L -o "$output" "$url"
else
wget -O "$output" "$url"
if ! curl -fL --retry 3 --retry-delay 3 --connect-timeout 10 --max-time 60 -o "$output" "$url"; then
log_error "Failed to download file from $url using curl"
exit 1
fi
else
if ! wget -O "$output" "$url"; then
log_error "Failed to download file from $url using wget"
exit 1
fi

Copilot uses AI. Check for mistakes.
Comment on lines 196 to 199
### API Keys

The installer automatically generates secure API keys in OpenAI format (`sk-...`). These keys are used for authenticating requests to your proxy server, **not** for provider authentication.
The installers automatically generate secure API keys in OpenAI format (`sk-...`). These keys are used for authenticating requests to your proxy server, **not** for provider authentication.

Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

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

The README states the installers generate “secure” API keys, but the new CLIProxyAPIPlus installer currently generates keys using Bash $RANDOM (not cryptographically secure). Either strengthen key generation in the installers or soften this wording to avoid overpromising security.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants