feat(cliproxyapi): enable api-keys authentication on Linux only - #461
Conversation
On Linux, uncomment and enable the api-keys section with CLIPROXY_API_KEY environment variable substitution. On macOS, keep api-keys commented for open access. 🤖 Generated with Claude Code by claude-opus-4-5-20250101
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Warning Rate limit exceeded@shunkakinoki has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 10 minutes and 41 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughUpdates the CLIPROXY API key placeholder in the configuration file and adds a Linux-specific post-template step to the startup script that uncomments and injects the CLIPROXY_API_KEY into the generated config.yaml file during initialization. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a feature to enable API key authentication for the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Mesa DescriptionTL;DREnables API key authentication for the 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 adds platform-specific logic to enable API key authentication for cliproxyapi on Linux, while leaving it open on macOS. The changes involve updating the YAML config template with a placeholder and adding a sed command to the start.sh script to activate the configuration on Linux. My review identifies a critical vulnerability in the new shell script logic. The sed command is susceptible to injection if the API key contains special characters, which could break the service startup. Additionally, the script enables authentication even with an empty API key, which is a potential security risk or misconfiguration. I have provided a detailed comment with a suggested code change to address these issues by adding a check for a non-empty key and properly escaping the key value before substitution.
| # Linux: uncomment and enable api-keys for client authentication | ||
| # macOS: leave api-keys commented for open access | ||
| if [ "$(uname)" = "Linux" ]; then | ||
| @sed@ -i \ | ||
| -e "s|^# api-keys:|api-keys:|" \ | ||
| -e "s|^# - \"__CLIPROXY_API_KEY__\"| - \"${CLIPROXY_API_KEY:-}\"|" \ | ||
| "$CONFIG" | ||
| fi |
There was a problem hiding this comment.
This block has two significant issues:
-
Empty API Key: It enables API key authentication even if
CLIPROXY_API_KEYis empty. This will result inapi-keys: [ "" ]in the configuration, which could be a security risk (allowing access with an empty key) or cause the service to fail if an empty key is invalid. Authentication should only be enabled when a valid key is present. -
sedInjection Vulnerability: Thesedcommand is vulnerable to script injection. IfCLIPROXY_API_KEYcontains characters that are special tosed's replacement string (e.g., the delimiter|,&, or\), the shell expansion will break thesedcommand or corrupt the key in the config file. This can prevent the service from starting.
Here is a more robust implementation that addresses both points by first checking for a non-empty key and then safely escaping it before substitution.
| # Linux: uncomment and enable api-keys for client authentication | |
| # macOS: leave api-keys commented for open access | |
| if [ "$(uname)" = "Linux" ]; then | |
| @sed@ -i \ | |
| -e "s|^# api-keys:|api-keys:|" \ | |
| -e "s|^# - \"__CLIPROXY_API_KEY__\"| - \"${CLIPROXY_API_KEY:-}\"|" \ | |
| "$CONFIG" | |
| fi | |
| # Linux: uncomment and enable api-keys for client authentication | |
| # macOS: leave api-keys commented for open access | |
| if [ "$(uname)" = "Linux" ] && [ -n "${CLIPROXY_API_KEY:-}" ]; then | |
| # Escape characters that are special to sed's replacement string: &, \, and the delimiter | | |
| safe_api_key=$(printf '%s' "$CLIPROXY_API_KEY" | @sed@ -e 's/[&|\\]/\\&/g') | |
| @sed@ -i \ | |
| -e "s|^# api-keys:|api-keys:|" \ | |
| -e "s|^# - \"__CLIPROXY_API_KEY__\"| - \"$safe_api_key\"|" \ | |
| "$CONFIG" | |
| fi |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [ "$(uname)" = "Linux" ]; then | ||
| @sed@ -i \ | ||
| -e "s|^# api-keys:|api-keys:|" \ | ||
| -e "s|^# - \"__CLIPROXY_API_KEY__\"| - \"${CLIPROXY_API_KEY:-}\"|" \ | ||
| "$CONFIG" |
There was a problem hiding this comment.
On Linux this block always uncomments api-keys and substitutes ${CLIPROXY_API_KEY:-}; when the env var is unset (e.g., missing .env in a systemd context), the config becomes api-keys: [""]. Depending on CLIProxyAPI’s auth logic, that either locks out all clients (no one has an empty key) or unintentionally allows empty-key access if a missing header is treated as "". To avoid surprising outages or false security, only enable the section when the key is non-empty or fail fast if it’s required.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the final PR Bugbot will review for you during this billing cycle
Your free Bugbot reviews will reset on January 27
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| if [ "$(uname)" = "Linux" ]; then | ||
| @sed@ -i \ | ||
| -e "s|^# api-keys:|api-keys:|" \ | ||
| -e "s|^# - \"__CLIPROXY_API_KEY__\"| - \"${CLIPROXY_API_KEY:-}\"|" \ |
There was a problem hiding this comment.
Empty API key enabled when environment variable unset
When CLIPROXY_API_KEY is not set or empty, the substitution ${CLIPROXY_API_KEY:-} results in an empty string being configured as a valid API key. On Linux, this produces a config with api-keys: [""], meaning the authentication system is enabled but with an empty string as the only valid key. Depending on how cliproxyapi validates API keys, this could either allow authentication with an empty API key header (security bypass) or block all legitimate requests (service outage). The script unconditionally uncomments the api-keys section without verifying the environment variable is set and non-empty.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="home-manager/services/cliproxyapi/scripts/start.sh">
<violation number="1" location="home-manager/services/cliproxyapi/scripts/start.sh:74">
P1: The API key authentication is enabled unconditionally on Linux without checking if `CLIPROXY_API_KEY` is actually set. When the environment variable is empty or unset, the config will contain `api-keys: [""]`, which could either lock out all clients (if empty keys are rejected) or create a security vulnerability (if empty keys are accepted). Add a check for a non-empty key value before enabling authentication.</violation>
</file>
Reply to cubic to teach it or ask questions. Tag @cubic-dev-ai to re-run a review.
|
|
||
| # Linux: uncomment and enable api-keys for client authentication | ||
| # macOS: leave api-keys commented for open access | ||
| if [ "$(uname)" = "Linux" ]; then |
There was a problem hiding this comment.
P1: The API key authentication is enabled unconditionally on Linux without checking if CLIPROXY_API_KEY is actually set. When the environment variable is empty or unset, the config will contain api-keys: [""], which could either lock out all clients (if empty keys are rejected) or create a security vulnerability (if empty keys are accepted). Add a check for a non-empty key value before enabling authentication.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/services/cliproxyapi/scripts/start.sh, line 74:
<comment>The API key authentication is enabled unconditionally on Linux without checking if `CLIPROXY_API_KEY` is actually set. When the environment variable is empty or unset, the config will contain `api-keys: [""]`, which could either lock out all clients (if empty keys are rejected) or create a security vulnerability (if empty keys are accepted). Add a check for a non-empty key value before enabling authentication.</comment>
<file context>
@@ -68,6 +68,15 @@ if [ -f "$TEMPLATE" ]; then
+
+ # Linux: uncomment and enable api-keys for client authentication
+ # macOS: leave api-keys commented for open access
+ if [ "$(uname)" = "Linux" ]; then
+ @sed@ -i \
+ -e "s|^# api-keys:|api-keys:|" \
</file context>
| if [ "$(uname)" = "Linux" ]; then | |
| if [ "$(uname)" = "Linux" ] && [ -n "${CLIPROXY_API_KEY:-}" ]; then |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
home-manager/services/cliproxyapi/scripts/start.sh (1)
72-79: Consider validating that CLIPROXY_API_KEY is set on Linux.The current implementation uses
${CLIPROXY_API_KEY:-}, which defaults to an empty string if the variable is unset. On Linux, this would generateapi-keys:with an empty list item- "", which may not provide the intended authentication behavior.🔎 Proposed validation to fail fast if the key is missing on Linux
# Linux: uncomment and enable api-keys for client authentication # macOS: leave api-keys commented for open access if [ "$(uname)" = "Linux" ]; then + if [ -z "${CLIPROXY_API_KEY:-}" ]; then + echo "ERROR: CLIPROXY_API_KEY must be set on Linux for client authentication" >&2 + exit 1 + fi @sed@ -i \ -e "s|^# api-keys:|api-keys:|" \ - -e "s|^# - \"__CLIPROXY_API_KEY__\"| - \"${CLIPROXY_API_KEY:-}\"|" \ + -e "s|^# - \"__CLIPROXY_API_KEY__\"| - \"${CLIPROXY_API_KEY}\"|" \ "$CONFIG" fi
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
config/cliproxyapi/config.yamlhome-manager/services/cliproxyapi/scripts/start.sh
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{json,yaml,yml,toml}
📄 CodeRabbit inference engine (.cursor/rules/formatting.mdc)
**/*.{json,yaml,yml,toml}: Use consistent indentation (2 spaces) in configuration files
Sort keys alphabetically when possible in configuration files
Use clear, descriptive names in configuration files
Files:
config/cliproxyapi/config.yaml
**/*.{yaml,yml,toml}
📄 CodeRabbit inference engine (.cursor/rules/formatting.mdc)
Add comments for complex configurations
Files:
config/cliproxyapi/config.yaml
**/*.{sh,bash}
📄 CodeRabbit inference engine (CLAUDE.md)
Use shfmt with 2-space indentation for shell scripts
**/*.{sh,bash}: Use 2 spaces for indentation in shell scripts
Add proper shebang lines to shell scripts
Follow shellcheck recommendations in shell scripts
Document complex commands in shell scripts
Use consistent variable naming in shell scripts
Files:
home-manager/services/cliproxyapi/scripts/start.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Cursor Bugbot
- GitHub Check: nix-nixos
- GitHub Check: nix-darwin
- GitHub Check: nix-linux
- GitHub Check: e2e-run (MacOS, macos-latest)
- GitHub Check: e2e-run (NixOS, ubuntu-latest)
- GitHub Check: e2e-run (Ubuntu, ubuntu-latest)
- GitHub Check: shellcheck
- GitHub Check: shellspec
- GitHub Check: docker-build-push (linux/arm64, arm64, ubuntu-24.04-arm)
- GitHub Check: docker-build-push (linux/amd64, amd64, ubuntu-latest)
- GitHub Check: lua-neovim
- GitHub Check: lua-neovim-test
- GitHub Check: lua-hammerspoon
🔇 Additional comments (2)
config/cliproxyapi/config.yaml (1)
18-18: LGTM! Placeholder aligns with the injection mechanism.The placeholder format is consistent with other configuration placeholders in the file (
__CLIPROXY_MANAGEMENT_PASSWORD__) and correctly matches the sed pattern in start.sh (line 77) that will uncomment and substitute this value on Linux.home-manager/services/cliproxyapi/scripts/start.sh (1)
74-79: Sed patterns work correctly and produce valid YAML; platform-specific behavior is properly implemented.The transformation successfully uncomments the api-keys section and replaces the placeholder on Linux. The generated config is valid YAML. The conditional at line 74 correctly limits the transformation to Linux only, leaving the api-keys commented on macOS for open access as intended.
Note: While the sed patterns currently match the template structure exactly, these patterns are inherently fragile. Consider documenting the expected template format or adding a validation step if the config template structure changes frequently.
Add ShellSpec tests to verify: - Linux: api-keys section is uncommented and CLIPROXY_API_KEY substituted - macOS: api-keys section remains commented for open access - Script contains the Linux-specific uncommenting logic 🤖 Generated with Claude Code by claude-opus-4-5-20250101
01210c1 to
46dae4e
Compare
1 participant
Changes
CLIPROXY_API_KEYsubstitution__CLIPROXY_API_KEY__placeholderTechnical Details
uname = Linux)# api-keys:and# - "__CLIPROXY_API_KEY__"linesCLIPROXY_API_KEYenvironment variableTesting
🤖 Generated with Claude Code by claude-opus-4-5-20250101
Note
Introduces platform-specific client authentication for CLIProxyAPI.
sedstep instart.shto uncommentapi-keysand substituteCLIPROXY_API_KEYinto the generatedconfig.yamlconfig/cliproxyapi/config.yamltemplate to include"__CLIPROXY_API_KEY__"placeholder under commentedapi-keysWritten by Cursor Bugbot for commit c0b61b8. Configure here.
Summary by cubic
Enable API key authentication on Linux by uncommenting the api-keys section and substituting CLIPROXY_API_KEY in the generated config.yaml. On macOS, api-keys remain commented for open access.
Written for commit fceb681. Summary will update automatically on new commits.