fix: guard cliproxyapi auth on restart - #545
Conversation
|
Warning Rate limit exceeded@shunkakinoki has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 21 minutes and 56 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 (4)
✨ Finishing touches🧪 Generate unit tests (beta)
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 critical fix for the Highlights
Ignored Files
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;DRAdded a pre-start S3 hydrate and re-sync guard for the What changed?
Description generated by Mesa. Update settings |
shunkakinoki
added
the
bug
![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 introduces a pre-start guard in the cliproxyapi start script to handle S3 authentication cache hydration and synchronization, preventing data loss on restart. The changes are logical and correctly implement the described workaround. I have one suggestion to refactor the new shell script logic to improve its readability and maintainability by reducing code duplication.
| if [ -n "$OBJECTSTORE_ENDPOINT" ] && [ -n "$OBJECTSTORE_ACCESS_KEY" ] && [ -n "$OBJECTSTORE_SECRET_KEY" ]; then | ||
| mkdir -p "$AUTH_DIR" | ||
|
|
||
| if [ -z "$(ls -A "$AUTH_DIR" 2>/dev/null)" ]; then | ||
| echo "⚠️ Local auth cache empty; hydrating from S3" >&2 | ||
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "s3://${OBJECTSTORE_BUCKET}/auths/" \ | ||
| "$AUTH_DIR/" || true | ||
|
|
||
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "s3://${OBJECTSTORE_BUCKET}/backup/auths/" \ | ||
| "$AUTH_DIR/" || true | ||
| fi | ||
|
|
||
| if [ -n "$(ls -A "$AUTH_DIR" 2>/dev/null)" ]; then | ||
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "$AUTH_DIR/" \ | ||
| "s3://${OBJECTSTORE_BUCKET}/auths/" || true | ||
|
|
||
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "$AUTH_DIR/" \ | ||
| "s3://${OBJECTSTORE_BUCKET}/backup/auths/" || true | ||
| fi | ||
| fi |
There was a problem hiding this comment.
The new logic for S3 synchronization contains significant code duplication, especially for the aws s3 sync commands and the AWS credentials. This can be refactored to improve readability and maintainability.
I suggest defining a helper function for the s3 sync operation and exporting the AWS credentials once at the beginning of the block. This will make the script cleaner, less error-prone, and easier to understand.
if [ -n "$OBJECTSTORE_ENDPOINT" ] && [ -n "$OBJECTSTORE_ACCESS_KEY" ] && [ -n "$OBJECTSTORE_SECRET_KEY" ]; then
export AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY"
export AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY"
s3_sync() {
@aws@ s3 sync --endpoint-url="$OBJECTSTORE_ENDPOINT" --no-progress "$@" || true
}
mkdir -p "$AUTH_DIR"
if [ -z "$(ls -A "$AUTH_DIR" 2>/dev/null)" ]; then
echo "⚠️ Local auth cache empty; hydrating from S3" >&2
s3_sync "s3://${OBJECTSTORE_BUCKET}/auths/" "$AUTH_DIR/"
s3_sync "s3://${OBJECTSTORE_BUCKET}/backup/auths/" "$AUTH_DIR/"
fi
if [ -n "$(ls -A "$AUTH_DIR" 2>/dev/null)" ]; then
s3_sync "$AUTH_DIR/" "s3://${OBJECTSTORE_BUCKET}/auths/"
s3_sync "$AUTH_DIR/" "s3://${OBJECTSTORE_BUCKET}/backup/auths/"
fi
fi![mesa-dot-dev[bot]](https://avatars.githubusercontent.com/in/1050077?s=60&v=4){kind=small}
There was a problem hiding this comment.
Performed full review of 41ef00a...fe4a9da
Analysis
-
Silent Error Handling: The implementation uses
|| truefor AWS CLI commands, which masks failures including authentication issues and network problems, creating debugging challenges and potential data loss when empty directories sync back to S3. -
Race Conditions: The guard logic lacks synchronization mechanisms, putting deployments at risk when multiple instances start simultaneously - they could corrupt data by racing to download and upload without coordination.
-
Data Consistency Risk: The re-sync logic runs regardless of hydration success, potentially overwriting valid S3 data with empty local directories. Additionally, without
--deleteflags, removed auth tokens remain in S3. -
Technical Debt: This "temporary workaround" lacks proper tracking mechanisms (TODO comments, follow-up tickets) to ensure it doesn't become permanent, creating future maintenance challenges.
-
Unexplained CI Changes: The artifact downgrade from v7 to v6 appears unrelated to the main fix and has no documented rationale, suggesting potential compatibility issues.
Tip
Help
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
0 files reviewed | 3 comments | Edit Agent Settings • Read Docs
| "$AUTH_DIR/" || true | ||
| fi | ||
|
|
||
| if [ -n "$(ls -A "$AUTH_DIR" 2>/dev/null)" ]; then |
There was a problem hiding this comment.
This condition checks if AUTH_DIR has files after the hydration attempts, but both hydration commands used || true (lines 41, 49). If both S3 syncs failed silently, this block will still execute and sync an empty directory back to S3, potentially overwriting valid remote data. Consider checking the exit status of the hydration commands before proceeding with the re-sync.
Agent: 🏛 Architecture •
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#545
File: home-manager/services/cliproxyapi/scripts/start.sh#L52
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
This condition checks if AUTH_DIR has files after the hydration attempts, but both hydration commands used `|| true` (lines 41, 49). If both S3 syncs failed silently, this block will still execute and sync an empty directory back to S3, potentially overwriting valid remote data. Consider checking the exit status of the hydration commands before proceeding with the re-sync.
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "s3://${OBJECTSTORE_BUCKET}/auths/" \ | ||
| "$AUTH_DIR/" || true |
There was a problem hiding this comment.
The || true here silently ignores all S3 sync failures, including authentication errors, network issues, or misconfigured endpoints. Consider logging the error and potentially failing fast if S3 credentials are provided but the sync fails, as this could indicate a configuration problem that will persist.
Agent: 🏛 Architecture •
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#545
File: home-manager/services/cliproxyapi/scripts/start.sh#L41
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
The `|| true` here silently ignores all S3 sync failures, including authentication errors, network issues, or misconfigured endpoints. Consider logging the error and potentially failing fast if S3 credentials are provided but the sync fails, as this could indicate a configuration problem that will persist.
| MANAGEMENT_PASSWORD="${CLIPROXY_MANAGEMENT_PASSWORD:-}" | ||
| export OBJECTSTORE_ENDPOINT OBJECTSTORE_BUCKET OBJECTSTORE_ACCESS_KEY OBJECTSTORE_SECRET_KEY MANAGEMENT_PASSWORD | ||
|
|
||
| if [ -n "$OBJECTSTORE_ENDPOINT" ] && [ -n "$OBJECTSTORE_ACCESS_KEY" ] && [ -n "$OBJECTSTORE_SECRET_KEY" ]; then |
There was a problem hiding this comment.
If multiple instances of this service start simultaneously (e.g., during a deployment rollout), they could all detect an empty AUTH_DIR and race to sync from/to S3, potentially causing conflicts or data corruption. Consider adding a lock mechanism or using S3's conditional operations to prevent concurrent modifications.
Agent: 🏛 Architecture •
Prompt for Agent
Task: Address review feedback left on GitHub.
Repository: shunkakinoki/dotfiles#545
File: home-manager/services/cliproxyapi/scripts/start.sh#L30
Action: Open this file location in your editor, inspect the highlighted code, and resolve the issue described below.
Feedback:
If multiple instances of this service start simultaneously (e.g., during a deployment rollout), they could all detect an empty AUTH_DIR and race to sync from/to S3, potentially causing conflicts or data corruption. Consider adding a lock mechanism or using S3's conditional operations to prevent concurrent modifications.
There was a problem hiding this comment.
Pull request overview
This PR implements a temporary workaround for a CLIProxyAPI auth cache issue by adding S3 hydration and re-sync guards directly in the start script, while also improving CI configuration.
Changes:
- Adds auth cache hydration and sync logic to start.sh to prevent auth loss on restart
- Wires awscli2 into the Nix start script via replaceVars
- Pins GitHub Actions download-artifact to v6 for compatibility with upload-artifact v6
- Adds nix-nixos job to the CI dependency check matrix
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| home-manager/services/cliproxyapi/scripts/start.sh | Adds AUTH_DIR definition and implements S3 hydration/sync guard logic when auth cache is empty |
| home-manager/services/cliproxyapi/default.nix | Wires awscli2 binary path into start script via replaceVars |
| .github/workflows/docker.yml | Pins download-artifact action to v6 for compatibility with upload-artifact v6 |
| .github/workflows/nix.yml | Adds nix-nixos job to the nix-check dependency list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "$AUTH_DIR/" \ | ||
| "s3://${OBJECTSTORE_BUCKET}/auths/" || true | ||
|
|
||
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "$AUTH_DIR/" \ | ||
| "s3://${OBJECTSTORE_BUCKET}/backup/auths/" || true |
There was a problem hiding this comment.
This duplicates the backup logic already present in backup.sh (lines 38-52). This creates maintenance burden as changes to the sync logic need to be made in multiple places. Consider calling the existing backup.sh script instead of duplicating the logic, or extract this into a shared function.
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | |
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | |
| @aws@ s3 sync \ | |
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | |
| --no-progress \ | |
| "$AUTH_DIR/" \ | |
| "s3://${OBJECTSTORE_BUCKET}/auths/" || true | |
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | |
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | |
| @aws@ s3 sync \ | |
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | |
| --no-progress \ | |
| "$AUTH_DIR/" \ | |
| "s3://${OBJECTSTORE_BUCKET}/backup/auths/" || true | |
| script_dir="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)" | |
| backup_script="${script_dir}/backup.sh" | |
| if [ ! -x "$backup_script" ]; then | |
| echo "Expected backup script not found or not executable: $backup_script" >&2 | |
| exit 1 | |
| fi | |
| OBJECTSTORE_ENDPOINT="$OBJECTSTORE_ENDPOINT" \ | |
| OBJECTSTORE_BUCKET="$OBJECTSTORE_BUCKET" \ | |
| OBJECTSTORE_ACCESS_KEY="$OBJECTSTORE_ACCESS_KEY" \ | |
| OBJECTSTORE_SECRET_KEY="$OBJECTSTORE_SECRET_KEY" \ | |
| AUTH_DIR="$AUTH_DIR" \ | |
| "$backup_script" |
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "s3://${OBJECTSTORE_BUCKET}/auths/" \ | ||
| "$AUTH_DIR/" || true | ||
|
|
||
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "s3://${OBJECTSTORE_BUCKET}/backup/auths/" \ | ||
| "$AUTH_DIR/" || true | ||
| fi | ||
|
|
||
| if [ -n "$(ls -A "$AUTH_DIR" 2>/dev/null)" ]; then | ||
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "$AUTH_DIR/" \ | ||
| "s3://${OBJECTSTORE_BUCKET}/auths/" || true | ||
|
|
||
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "$AUTH_DIR/" \ | ||
| "s3://${OBJECTSTORE_BUCKET}/backup/auths/" || true |
There was a problem hiding this comment.
The || true at the end of each aws s3 sync command ensures the script continues even if syncs fail. While this provides resilience, failed syncs are completely silent with no error logging. Consider adding error logging after failed syncs to aid debugging, for example: || echo "⚠️ Failed to sync from/to S3 [location]" >&2
| if [ -n "$(ls -A "$AUTH_DIR" 2>/dev/null)" ]; then | ||
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "$AUTH_DIR/" \ | ||
| "s3://${OBJECTSTORE_BUCKET}/auths/" || true | ||
|
|
||
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "$AUTH_DIR/" \ | ||
| "s3://${OBJECTSTORE_BUCKET}/backup/auths/" || true | ||
| fi |
There was a problem hiding this comment.
On every restart when the auth cache is not empty, four S3 sync operations are performed (lines 55-67). This happens even if the local and remote are already in sync. The aws s3 sync command is efficient and only uploads changed files, but the overhead of establishing connections and checking every file on each restart could add startup latency. Consider adding a timestamp check or other mechanism to skip syncing if a recent sync has already occurred.
| if [ -n "$OBJECTSTORE_ENDPOINT" ] && [ -n "$OBJECTSTORE_ACCESS_KEY" ] && [ -n "$OBJECTSTORE_SECRET_KEY" ]; then | ||
| mkdir -p "$AUTH_DIR" | ||
|
|
||
| if [ -z "$(ls -A "$AUTH_DIR" 2>/dev/null)" ]; then | ||
| echo "⚠️ Local auth cache empty; hydrating from S3" >&2 | ||
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "s3://${OBJECTSTORE_BUCKET}/auths/" \ | ||
| "$AUTH_DIR/" || true | ||
|
|
||
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "s3://${OBJECTSTORE_BUCKET}/backup/auths/" \ | ||
| "$AUTH_DIR/" || true | ||
| fi | ||
|
|
||
| if [ -n "$(ls -A "$AUTH_DIR" 2>/dev/null)" ]; then | ||
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "$AUTH_DIR/" \ | ||
| "s3://${OBJECTSTORE_BUCKET}/auths/" || true | ||
|
|
||
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "$AUTH_DIR/" \ | ||
| "s3://${OBJECTSTORE_BUCKET}/backup/auths/" || true | ||
| fi | ||
| fi |
There was a problem hiding this comment.
The new auth guard logic (lines 30-69) lacks test coverage. While hydrate.sh and backup.sh have tests in spec/cliproxyapi_backup_spec.sh, the conditional logic for checking if AUTH_DIR is empty, hydrating, and syncing back should be tested. Consider adding test cases to spec/cliproxyapi_spec.sh that verify: 1) hydration occurs when AUTH_DIR is empty, 2) hydration is skipped when AUTH_DIR has files, 3) sync back occurs after hydration, and 4) proper handling when S3 credentials are missing.
| MANAGEMENT_PASSWORD="${CLIPROXY_MANAGEMENT_PASSWORD:-}" | ||
| export OBJECTSTORE_ENDPOINT OBJECTSTORE_BUCKET OBJECTSTORE_ACCESS_KEY OBJECTSTORE_SECRET_KEY MANAGEMENT_PASSWORD | ||
|
|
||
| if [ -n "$OBJECTSTORE_ENDPOINT" ] && [ -n "$OBJECTSTORE_ACCESS_KEY" ] && [ -n "$OBJECTSTORE_SECRET_KEY" ]; then |
There was a problem hiding this comment.
The condition should also check that OBJECTSTORE_BUCKET is not empty. While it defaults to "cliproxyapi" on line 24, it's possible for the variable to be explicitly set to an empty string in the environment, which would result in invalid S3 paths like "s3:///auths/".
| if [ -n "$OBJECTSTORE_ENDPOINT" ] && [ -n "$OBJECTSTORE_ACCESS_KEY" ] && [ -n "$OBJECTSTORE_SECRET_KEY" ]; then | |
| if [ -n "$OBJECTSTORE_ENDPOINT" ] && [ -n "$OBJECTSTORE_ACCESS_KEY" ] && [ -n "$OBJECTSTORE_SECRET_KEY" ] && [ -n "$OBJECTSTORE_BUCKET" ]; then |
| if [ -z "$(ls -A "$AUTH_DIR" 2>/dev/null)" ]; then | ||
| echo "⚠️ Local auth cache empty; hydrating from S3" >&2 | ||
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "s3://${OBJECTSTORE_BUCKET}/auths/" \ | ||
| "$AUTH_DIR/" || true | ||
|
|
||
| AWS_ACCESS_KEY_ID="$OBJECTSTORE_ACCESS_KEY" \ | ||
| AWS_SECRET_ACCESS_KEY="$OBJECTSTORE_SECRET_KEY" \ | ||
| @aws@ s3 sync \ | ||
| --endpoint-url="$OBJECTSTORE_ENDPOINT" \ | ||
| --no-progress \ | ||
| "s3://${OBJECTSTORE_BUCKET}/backup/auths/" \ | ||
| "$AUTH_DIR/" || true | ||
| fi |
There was a problem hiding this comment.
This duplicates the hydration logic already present in hydrate.sh (lines 33-47). This creates maintenance burden as changes to the sync logic need to be made in multiple places. Consider calling the existing hydrate.sh script instead of duplicating the logic, or extract this into a shared function.
![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 4 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:31">
P2: Auth cache directory is created/populated without restrictive permissions, leaving OAuth tokens potentially world-readable</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| export OBJECTSTORE_ENDPOINT OBJECTSTORE_BUCKET OBJECTSTORE_ACCESS_KEY OBJECTSTORE_SECRET_KEY MANAGEMENT_PASSWORD | ||
|
|
||
| if [ -n "$OBJECTSTORE_ENDPOINT" ] && [ -n "$OBJECTSTORE_ACCESS_KEY" ] && [ -n "$OBJECTSTORE_SECRET_KEY" ]; then | ||
| mkdir -p "$AUTH_DIR" |
There was a problem hiding this comment.
P2: Auth cache directory is created/populated without restrictive permissions, leaving OAuth tokens potentially world-readable
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 31:
<comment>Auth cache directory is created/populated without restrictive permissions, leaving OAuth tokens potentially world-readable</comment>
<file context>
@@ -26,6 +27,47 @@ OBJECTSTORE_SECRET_KEY="$(strip_quotes "${OBJECTSTORE_SECRET_KEY:-}")"
export OBJECTSTORE_ENDPOINT OBJECTSTORE_BUCKET OBJECTSTORE_ACCESS_KEY OBJECTSTORE_SECRET_KEY MANAGEMENT_PASSWORD
+if [ -n "$OBJECTSTORE_ENDPOINT" ] && [ -n "$OBJECTSTORE_ACCESS_KEY" ] && [ -n "$OBJECTSTORE_SECRET_KEY" ]; then
+ mkdir -p "$AUTH_DIR"
+
+ if [ -z "$(ls -A "$AUTH_DIR" 2>/dev/null)" ]; then
</file context>
Changes
Technical Details
s3://$OBJECTSTORE_BUCKET/authsandbackup/auths, then re-sync back to S3 to avoid deletions on restartTesting
🤖 Generated with Codex CLI by GPT-5
Summary by cubic
Protect CLIProxyAPI auth cache on restart by hydrating from S3 when the local cache is empty, then re-syncing to avoid deletions. Also wires awscli into the start script and makes small CI tweaks.
Bug Fixes
Dependencies
Written for commit fe4a9da. Summary will update on new commits.