fix(cliproxyapi): read API key from .env and skip dotfiles sync on Linux - #462
Conversation
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on January 27. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
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. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughPlatform-specific bootstrap and auth sync behavior were restricted to macOS; Linux no longer bootstraps/syncs dotfiles when objectstore is empty. The Linux startup now only injects CLIPROXY_API_KEY when it is non-empty. Example env and tests were updated accordingly; a path watch in default.nix was reduced. Changes
Sequence Diagram(s)(Skipped — changes are localized platform guards and conditional injections without a new multi-component sequential flow that requires visualization.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{sh,bash}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
⏰ 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). (13)
🔇 Additional comments (2)
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 refines the API key handling 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
|
- Only enable api-keys auth on Linux when CLIPROXY_API_KEY is set - Prevents empty string "" in api-keys when env var is missing - Add CLIPROXY_API_KEY to .env.example - Add test for empty key behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Mesa DescriptionTL;DRFixes What changed?
Description generated by Mesa. Update settings |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![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 correctly ensures that API key authentication for cliproxyapi on Linux is only enabled when CLIPROXY_API_KEY is set, preventing issues with empty keys. The changes to .env.example and the new test case for an empty key are good additions.
My review includes one high-severity finding in start.sh where an API key with special characters could break the sed command. I've provided a code suggestion to properly escape the key before its use. I've also added a medium-severity comment to the test file, recommending a new test case to cover this special character scenario to make the test suite more robust.
| @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" |
There was a problem hiding this comment.
The CLIPROXY_API_KEY variable is used directly in a sed replacement string. If the API key contains special characters for sed (like &, \, or the | delimiter), it can break the substitution command or lead to unexpected results. It's safer to escape the variable before using it.
For example, if CLIPROXY_API_KEY was foo&bar, the & in the replacement part would be substituted with the entire matched pattern (# - "__CLIPROXY_API_KEY__"), which is not the desired behavior.
| @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" | |
| ESCAPED_API_KEY=$(printf '%s\n' "${CLIPROXY_API_KEY}" | @sed@ -e 's/[&\\|]/\\&/g') | |
| @sed@ -i \ | |
| -e "s|^# api-keys:|api-keys:|" \ | |
| -e "s|^# - \"__CLIPROXY_API_KEY__\"| - \"$ESCAPED_API_KEY\"|" "$CONFIG" |
| It 'keeps api-keys commented on Linux when CLIPROXY_API_KEY is empty' | ||
| # Create test script that simulates Linux behavior with empty key | ||
| cat >"$TEMP_HOME/test_linux_empty_apikey.sh" <<'EOF' | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
| CONFIG_DIR="$HOME/.cli-proxy-api" | ||
| TEMPLATE="$CONFIG_DIR/config.template.yaml" | ||
| CONFIG="$CONFIG_DIR/config.yaml" | ||
| CLIPROXY_API_KEY="" | ||
|
|
||
| # Copy template to config | ||
| cp "$TEMPLATE" "$CONFIG" | ||
|
|
||
| # Simulate Linux behavior with empty key (should NOT uncomment) | ||
| if [ -n "${CLIPROXY_API_KEY:-}" ]; then | ||
| sed \ | ||
| -e "s|^# api-keys:|api-keys:|" \ | ||
| -e "s|^# - \"__CLIPROXY_API_KEY__\"| - \"${CLIPROXY_API_KEY}\"|" \ | ||
| "$CONFIG" > "$CONFIG.tmp" && mv "$CONFIG.tmp" "$CONFIG" | ||
| fi | ||
|
|
||
| cat "$CONFIG" | ||
| EOF | ||
| chmod +x "$TEMP_HOME/test_linux_empty_apikey.sh" | ||
|
|
||
| When run bash -c "HOME='$TEMP_HOME' bash '$TEMP_HOME/test_linux_empty_apikey.sh'" | ||
| The output should include '# api-keys:' | ||
| The output should include '# - "__CLIPROXY_API_KEY__"' | ||
| The status should be success | ||
| End |
There was a problem hiding this comment.
This is a great test for the empty API key case! To make the tests more robust, consider adding another test case for when CLIPROXY_API_KEY contains special characters that have meaning in sed replacement strings (e.g., &, \, |). This would help ensure the script is resilient against a wider range of possible key values, especially with the proposed fix in start.sh.
shunkakinoki
changed the title
On Linux, auth files are now only synced between R2, ~/.cli-proxy-api/objectstore/auths/, and ~/.ccs/cliproxy/auth/. The redundant copy to ~/dotfiles/objectstore/auths/ is skipped to avoid duplicate files. macOS behavior is preserved - auth files still sync to dotfiles for git tracking. Changes: - start.sh: Bootstrap from dotfiles is now Darwin-only - backup-auth.sh: Sync to dotfiles is now Darwin-only - default.nix: Removed dotfiles path from Linux systemd path watcher 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
CLIPROXY_API_KEYfrom.envfile for API authentication on Linux~/dotfiles/objectstore/auths/on Linux (macOS behavior preserved)Changes
API Key from .env
.envfile to getCLIPROXY_API_KEYSkip dotfiles sync on Linux
start.sh: Bootstrap from dotfiles is now Darwin-onlybackup-auth.sh: Sync to dotfiles is now Darwin-onlydefault.nix: Removed dotfiles path from Linux systemd path watcherOn Linux, auth files now only sync between:
~/.cli-proxy-api/objectstore/auths/~/.ccs/cliproxy/auth/Test plan
make formatpassesmake shell-testpasses (242 examples, 0 failures)🤖 Generated with Claude Code