-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor SSH agent keychain integration for Linux #426
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
c0e7856
0ad2cb4
9aba5aa
6a5b1e7
abf72eb
37f5b01
75b33e7
4a3cd99
afbcf7a
3523468
dc1fb7b
7c93d29
82dda83
9a5171f
4cf0463
f984462
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| #!/usr/bin/env bash | ||
| # Combined backup and recovery script | ||
| # Backs up auth files to R2, then recovers if missing | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
|
|
||
| # Source .env for credentials | ||
| if [ -f "$HOME/dotfiles/.env" ]; then | ||
| set -a | ||
| # shellcheck source=/dev/null | ||
| source "$HOME/dotfiles/.env" | ||
| set +a | ||
| fi | ||
|
|
||
| # Run backup | ||
| echo "[$(date)] Starting backup..." | ||
| "$SCRIPT_DIR/backup-auth.sh" | ||
|
|
||
| # Run recovery if needed | ||
| echo "[$(date)] Checking for recovery..." | ||
| "$SCRIPT_DIR/recover-auth.sh" | ||
|
|
||
| echo "[$(date)] Backup/recovery cycle complete" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| #!/usr/bin/env bash | ||
| # Backup auth files to R2 before service start | ||
| # Protects against race condition deletions | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| CONFIG_DIR="$HOME/.cli-proxy-api" | ||
| BACKUP_DIR="s3://cliproxyapi/backup/auths/" | ||
| AUTH_DIR="$CONFIG_DIR/objectstore/auths" | ||
|
|
||
| # Check if auth directory has files | ||
| if [ -d "$AUTH_DIR" ] && [ -n "$(ls -A "$AUTH_DIR" 2>/dev/null)" ]; then | ||
| echo "Backing up auth files to R2 backup directory..." >&2 | ||
| 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/" \ | ||
| "$BACKUP_DIR" 2>/dev/null || echo "⚠️ Backup failed (continuing anyway)" >&2 | ||
| fi |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| #!/usr/bin/env bash | ||
| # Recover auth files from backup if missing | ||
| # Handles race condition where files get deleted during config reload | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| CONFIG_DIR="$HOME/.cli-proxy-api" | ||
| BACKUP_DIR="s3://cliproxyapi/backup/auths/" | ||
| AUTH_DIR="$CONFIG_DIR/objectstore/auths" | ||
|
|
||
| # Check if auth directory is missing or empty | ||
| if [ ! -d "$AUTH_DIR" ] || [ -z "$(ls -A "$AUTH_DIR" 2>/dev/null)" ]; then | ||
| echo "Auth files missing, attempting recovery from R2 backup..." >&2 | ||
| mkdir -p "$AUTH_DIR" | ||
| AWS_ACCESS_KEY_ID="${OBJECTSTORE_ACCESS_KEY}" \ | ||
| AWS_SECRET_ACCESS_KEY="${OBJECTSTORE_SECRET_KEY}" \ | ||
| aws s3 sync \ | ||
| --endpoint-url="${OBJECTSTORE_ENDPOINT}" \ | ||
| --no-progress \ | ||
| "$BACKUP_DIR" \ | ||
| "$AUTH_DIR/" 2>/dev/null && echo "✅ Recovered auth files from backup" >&2 || echo "⚠️ Recovery failed (no backup available?)" >&2 | ||
|
Comment on lines
+13
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: find . -name "recover-auth.sh" -type fRepository: shunkakinoki/dotfiles Length of output: 124 🏁 Script executed: cat -n ./home-manager/services/cliproxyapi/scripts/recover-auth.shRepository: shunkakinoki/dotfiles Length of output: 1050 Verify aws CLI availability before use. The script uses the 🤖 Prompt for AI Agents |
||
| fi | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,16 +18,11 @@ in | |
| # Load keys that exist | ||
| set -l keys | ||
|
|
||
| # Always try to load the default key (no passphrase) | ||
| # Always try to load the default key | ||
| if test -f ~/.ssh/id_ed25519 | ||
| set -a keys ~/.ssh/id_ed25519 | ||
| end | ||
|
Comment on lines
+21
to
24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # Search for references to id_ed25519_github in the codebase
rg "id_ed25519_github" -n --type-list | head -5
rg "id_ed25519_github" -n 2>/dev/null || echo "No references found"Repository: shunkakinoki/dotfiles Length of output: 2153 🏁 Script executed: # Check for SSH identity configurations that might reference this key pattern
rg "IdentityFile|identity.*github|ssh.*github" -n -g "*.nix" -g "*.conf" -g "*.ssh" 2>/dev/null | head -20Repository: shunkakinoki/dotfiles Length of output: 941 🏁 Script executed: # Get full context of the ssh-agent service configuration
if [ -f "home-manager/services/ssh-agent/default.nix" ]; then
wc -l "home-manager/services/ssh-agent/default.nix"
cat -n "home-manager/services/ssh-agent/default.nix"
fiRepository: shunkakinoki/dotfiles Length of output: 1325 This removal is a breaking change—verify user migration path or update documentation. The removal of automatic While an on-demand mechanism exists via the
|
||
|
|
||
| # Load GitHub key if it exists (may have passphrase) | ||
| if test -f ~/.ssh/id_ed25519_github | ||
| set -a keys ~/.ssh/id_ed25519_github | ||
| end | ||
|
|
||
| # Initialize keychain with found keys | ||
| if test (count $keys) -gt 0 | ||
| # Use --quiet to suppress most output, --eval to set environment variables | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Add service dependencies to prevent coordination issues.
The backup service and timer lack explicit dependencies on the main
cliproxyapi.service. This could lead to:Based on learnings, service configurations should include proper service dependencies.
Recommended fix to add proper service ordering
systemd.user.services.cliproxyapi-backup = lib.mkIf pkgs.stdenv.isLinux { Unit = { Description = "CLIProxyAPI auth backup and recovery"; + After = [ "cliproxyapi.service" ]; }; Service = { Type = "oneshot"; ExecStart = "${pkgs.bash}/bin/bash ${./scripts/backup-and-recover.sh}"; }; };This ensures the backup service runs after the main service is active, reducing the risk of file access conflicts.
📝 Committable suggestion