-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/declarative ssh agent #417
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
e3c9aa8
d4a266f
a10df99
1b3c7a3
cd8102e
610d5c4
53efff6
d2424a9
5f87a2f
2877d52
39339a5
d3f6500
1605c8e
aa1ac66
9e55ccb
88ad7a4
1311a1b
d84694f
46edf4e
9d97d9b
f67ae81
d21c035
41a1601
45ad715
67f5d26
ba04595
a7f9718
0069080
f09fb3c
08ccad0
5ca4f57
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 |
|---|---|---|
|
|
@@ -86,6 +86,7 @@ with pkgs; | |
| docker | ||
| docker-compose | ||
| gemini-cli | ||
| keychain | ||
| opencode | ||
| powertop | ||
| tailscale | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,40 @@ | ||||||||||||||||||||||||||||||||||||
| function _ssh_add_github --description "Add GitHub SSH key to ssh-agent" | ||||||||||||||||||||||||||||||||||||
| # Check if key exists | ||||||||||||||||||||||||||||||||||||
| if not test -f ~/.ssh/id_ed25519_github | ||||||||||||||||||||||||||||||||||||
| echo "❌ GitHub SSH key not found at ~/.ssh/id_ed25519_github" | ||||||||||||||||||||||||||||||||||||
| return 1 | ||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Check if keychain is available | ||||||||||||||||||||||||||||||||||||
| if not command -v keychain >/dev/null | ||||||||||||||||||||||||||||||||||||
| echo "❌ keychain not found. Please install keychain." | ||||||||||||||||||||||||||||||||||||
| return 1 | ||||||||||||||||||||||||||||||||||||
| end | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Initialize keychain and add the GitHub key | ||||||||||||||||||||||||||||||||||||
| echo "🔑 Adding GitHub SSH key to keychain..." | ||||||||||||||||||||||||||||||||||||
| # Use bash to evaluate keychain output, then use ssh-add | ||||||||||||||||||||||||||||||||||||
| bash -c 'eval $(keychain --eval --quiet --confirm ~/.ssh/id_ed25519_github 2>/dev/null); ssh-add -l' >/dev/null 2>&1 | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Alternative: directly use ssh-add if keychain already initialized ssh-agent | ||||||||||||||||||||||||||||||||||||
| if not ssh-add -l >/dev/null 2>&1 | ||||||||||||||||||||||||||||||||||||
| # Start ssh-agent if not running | ||||||||||||||||||||||||||||||||||||
| eval (ssh-agent -c) | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
| eval (ssh-agent -c) | |
| eval (ssh-agent | psub) |
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.
The logic for adding the SSH key is overly complex and contains a critical error. The command on line 17, bash -c 'eval $(keychain ...)', is incorrect for a fish shell. The environment variables set by eval will be scoped to the bash subshell and won't be available in the parent fish shell, making the command ineffective.
The subsequent logic (lines 20-26) correctly checks for and starts ssh-agent if needed, and then adds the key with ssh-add. This is a robust approach.
I suggest simplifying the script to remove the confusing and non-functional keychain call. The remaining logic is sufficient to add the key, prompting for a passphrase if necessary. While this makes the keychain availability check on line 9 redundant for this specific function, keychain is still used for agent initialization at login, so ssh-add should work correctly here.
# Ensure ssh-agent is running. keychain should have started it on login,
# but we can start it here as a fallback if needed.
if not ssh-add -l >/dev/null 2>&1
echo "ssh-agent not running. Starting it..."
eval (ssh-agent -c)
end
# Add the key directly. This will prompt for a passphrase if there is one.
echo "🔑 Adding GitHub SSH key to ssh-agent..."
ssh-add ~/.ssh/id_ed25519_github
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
Simplify redundant SSH agent initialization flow.
The function has confusing redundant logic:
- Line 17 uses
bashto evaluate keychain (then ignores the result) - Lines 20-23 check and potentially start ssh-agent
- Line 26 directly adds the key
This creates unnecessary complexity. Since keychain is already initialized in loginShellInit (from ssh-agent.nix), the function should verify the agent is running and add the key only if not already present.
Apply this diff to simplify:
- # Initialize keychain and add the GitHub key
- echo "🔑 Adding GitHub SSH key to keychain..."
- # Use bash to evaluate keychain output, then use ssh-add
- bash -c 'eval $(keychain --eval --quiet --confirm ~/.ssh/id_ed25519_github 2>/dev/null); ssh-add -l' >/dev/null 2>&1
-
- # Alternative: directly use ssh-add if keychain already initialized ssh-agent
+ # Check if ssh-agent is running (keychain should have started it)
if not ssh-add -l >/dev/null 2>&1
- # Start ssh-agent if not running
- eval (ssh-agent -c)
+ echo "❌ ssh-agent not running. Run 'eval (keychain --eval)' first."
+ return 1
end
+ # Check if key is already added
+ if ssh-add -l | grep -qE "(id_ed25519_github|shunkakinoki@gmail.com)"
+ echo "✅ GitHub SSH key already loaded"
+ return 0
+ end
+
+ # Add the key
+ echo "🔑 Adding GitHub SSH key..."
ssh-add ~/.ssh/id_ed25519_githubCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In home-manager/programs/fish/functions/_ssh_add_github.fish around lines 17 to
26, remove the redundant bash keychain eval and the manual ssh-agent startup;
instead simply check whether the github key is already loaded and, if not, call
ssh-add to add it. Implement this by running ssh-add -l and testing for the
presence of ~/.ssh/id_ed25519_github (or its key fingerprint/name) and only call
ssh-add ~/.ssh/id_ed25519_github when that check fails; do not attempt to start
ssh-agent or invoke keychain here since agent initialization is handled in
loginShellInit.
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.
🧩 Analysis chain
🏁 Script executed:
cat -n home-manager/programs/fish/functions/_ssh_add_github.fish | head -40Repository: shunkakinoki/dotfiles
Length of output: 1797
🌐 Web query:
ssh-add -l output format what fields does it show
💡 Result:
ssh-add -l prints one line per identity with these fields:
- bits — key size (e.g. 4096)
- fingerprint — fingerprint of the public key (default SHA256:... unless changed with -E)
- comment — free‑form comment (usually user@host or whatever was set in the pubkey); ssh-agent does not record the source filename.
- algorithm — key type in parentheses (e.g. (RSA), (ED25519))
Example:
4096 SHA256:abc... user@host (RSA). (mankier.com)
Citations:
The grep pattern is ineffective; verification relies entirely on the hardcoded email.
The ssh-add -l command does not output the filename id_ed25519_github; it only shows the comment field (usually user@host format). This means the first pattern in the regex will never match. The verification succeeds only if the public key's comment is exactly shunkakinoki@gmail.com. If the key was generated with a different comment (e.g., shun@laptop or user@hostname), verification will fail despite the key being correctly added.
Either document that the key must be generated with this specific comment, or use a more robust check (e.g., verify by fingerprint or check ssh-add -L for the full key).
🤖 Prompt for AI Agents
In home-manager/programs/fish/functions/_ssh_add_github.fish around line 29, the
current grep against ssh-add -l relies on a filename that never appears and on a
hardcoded email comment; replace this with a robust check using ssh-add -L
(which prints the public keys) and match either the public key blob or the key
fingerprint (or both) instead of the comment alone, e.g., call ssh-keygen -lf on
each public key line from ssh-add -L to compare the expected fingerprint, or
grep the full public key string/comment from ssh-add -L; alternatively, if you
prefer keeping the simple approach, update the function comment to require
generating the key with the specific email and document that constraint.
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.
Fallback error handling could be more robust.
The fallback path (lines 34-39) returns the status of ssh -T git@github.com, but this command always exits with status 1 for successful authentication (GitHub's design). This makes the return value misleading.
Apply this diff to handle GitHub's authentication response correctly:
else
echo "⚠️ Could not verify key was added, but ssh-add may have succeeded"
echo "🧪 Testing GitHub connection anyway..."
- ssh -T git@github.com
- return $status
+ # GitHub returns exit code 1 for successful auth with message
+ # "Hi username! You've successfully authenticated..."
+ if ssh -T git@github.com 2>&1 | grep -q "successfully authenticated"
+ return 0
+ else
+ return 1
+ end
end📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| else | |
| echo "⚠️ Could not verify key was added, but ssh-add may have succeeded" | |
| echo "🧪 Testing GitHub connection anyway..." | |
| ssh -T git@github.com | |
| return $status | |
| end | |
| else | |
| echo "⚠️ Could not verify key was added, but ssh-add may have succeeded" | |
| echo "🧪 Testing GitHub connection anyway..." | |
| # GitHub returns exit code 1 for successful auth with message | |
| # "Hi username! You've successfully authenticated..." | |
| if ssh -T git@github.com 2>&1 | grep -q "successfully authenticated" | |
| return 0 | |
| else | |
| return 1 | |
| end | |
| end |
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,40 @@ | ||||||||||||||||
| { | ||||||||||||||||
| config, | ||||||||||||||||
| lib, | ||||||||||||||||
| pkgs, | ||||||||||||||||
| ... | ||||||||||||||||
| }: | ||||||||||||||||
| let | ||||||||||||||||
| inherit (pkgs.stdenv) isLinux isDarwin; | ||||||||||||||||
| in | ||||||||||||||||
| { | ||||||||||||||||
| # Configure keychain for Linux systems | ||||||||||||||||
| # macOS uses built-in keychain via UseKeychain SSH option | ||||||||||||||||
| programs.fish.loginShellInit = lib.mkIf isLinux ( | ||||||||||||||||
| lib.mkAfter '' | ||||||||||||||||
| # Initialize keychain for SSH key management | ||||||||||||||||
| # This automatically starts ssh-agent and loads SSH keys | ||||||||||||||||
| if command -v keychain > /dev/null | ||||||||||||||||
| # Load keys that exist | ||||||||||||||||
| set -l keys | ||||||||||||||||
|
|
||||||||||||||||
| # Always try to load the default key (no passphrase) | ||||||||||||||||
| if test -f ~/.ssh/id_ed25519 | ||||||||||||||||
| set -a keys ~/.ssh/id_ed25519 | ||||||||||||||||
| end | ||||||||||||||||
|
|
||||||||||||||||
| # 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 | ||||||||||||||||
| # --confirm will skip keys that need a passphrase in non-interactive contexts | ||||||||||||||||
| eval (keychain --eval --quiet --confirm $keys ^/dev/null; or true) | ||||||||||||||||
|
Comment on lines
+34
to
+35
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🌐 Web query:
💡 Result: Short answer: --confirm tells keychain to add keys with SSH confirmation enabled (ssh-add -c). That makes every use require an interactive confirmation via SSH_ASKPASS; in non‑interactive contexts (cron, CI, headless shells without SSH_ASKPASS) the key will either block waiting for confirmation or fail, so it is unsuitable for non‑interactive use. [1][2] What to do
Sources
Remove The 🤖 Prompt for AI AgentsThere 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. Error suppression may hide legitimate keychain failures. The Consider logging suppressed errors to a file for debugging: - eval (keychain --eval --quiet --confirm $keys ^/dev/null; or true)
+ eval (keychain --eval --quiet --confirm $keys 2>>~/.keychain-errors.log; or true)Or at minimum, document why errors are suppressed: # Use --quiet to suppress most output, --eval to set environment variables
- # --confirm will skip keys that need a passphrase in non-interactive contexts
+ # --confirm will skip keys that need a passphrase in non-interactive contexts
+ # Errors are suppressed to prevent login shell failures if keychain has issues
eval (keychain --eval --quiet --confirm $keys ^/dev/null; or true)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||
| eval (keychain --eval --quiet --confirm $keys ^/dev/null; or true) | |
| eval (keychain --eval --quiet --confirm $keys ^/dev/null); or true |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,13 +11,14 @@ let | |||||||||
| in | ||||||||||
| { | ||||||||||
| # Shared SSH key for GitHub authentication (accessible on all machines) | ||||||||||
| "keys/id_ed25519.age" = { | ||||||||||
| file = ./keys/id_ed25519.age; | ||||||||||
| # This is ~/.ssh/id_github on galactica, the GitHub CLI-authorized key | ||||||||||
| "keys/id_github.age" = { | ||||||||||
| file = ./keys/id_github.age; | ||||||||||
|
Comment on lines
+15
to
+16
|
||||||||||
| "keys/id_github.age" = { | |
| file = ./keys/id_github.age; | |
| "keys/id_ed25519.age" = { | |
| file = ./keys/id_ed25519.age; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -49,11 +49,19 @@ Once Tailscale is set up: | |||||||||||||||||||||||
| kyber # Fish abbreviation that runs: ssh ubuntu@kyber | ||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ## Syncing SSH Keys from Galactica | ||||||||||||||||||||||||
| ## SSH Key Management | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| To sync the GitHub SSH key from galactica to kyber: | ||||||||||||||||||||||||
| ### Automated Setup | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ### On Galactica | ||||||||||||||||||||||||
| This configuration uses: | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| - **agenix**: Encrypts and syncs the GitHub SSH key from galactica | ||||||||||||||||||||||||
| - **keychain**: Manages ssh-agent and automatically loads SSH keys | ||||||||||||||||||||||||
| - **Declarative deployment**: SSH keys are deployed during `make switch` | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ### Syncing SSH Keys from Galactica | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| #### On Galactica (one-time setup) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||||
| cd ~/dotfiles | ||||||||||||||||||||||||
|
|
@@ -64,15 +72,58 @@ git commit -m "chore(agenix): rekey secrets for kyber access" | |||||||||||||||||||||||
| git push | ||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ### On Kyber | ||||||||||||||||||||||||
| #### On Kyber | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||||
| cd ~/dotfiles | ||||||||||||||||||||||||
| git pull | ||||||||||||||||||||||||
| make switch | ||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| The GitHub SSH key will be automatically: | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| 1. Decrypted from `named-hosts/galactica/keys/id_ed25519.age` | ||||||||||||||||||||||||
|
Contributor
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. The secret file for the GitHub SSH key was renamed from
Suggested change
|
||||||||||||||||||||||||
| 1. Decrypted from `named-hosts/galactica/keys/id_ed25519.age` | |
| 1. Decrypted from `named-hosts/galactica/keys/id_github.age` |
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.
Documentation references outdated filename.
Line 85 references id_ed25519.age but the secrets configuration uses id_github.age. Update to match the actual filename.
The GitHub SSH key will be automatically:
-1. Decrypted from `named-hosts/galactica/keys/id_ed25519.age`
+1. Decrypted from `named-hosts/galactica/keys/id_github.age`
2. Deployed to `~/.ssh/id_ed25519_github`
3. Loaded into ssh-agent via keychain (if no passphrase)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| The GitHub SSH key will be automatically: | |
| 1. Decrypted from `named-hosts/galactica/keys/id_ed25519.age` | |
| 2. Deployed to `~/.ssh/id_ed25519_github` | |
| 3. Loaded into ssh-agent via keychain (if no passphrase) | |
| The GitHub SSH key will be automatically: | |
| 1. Decrypted from `named-hosts/galactica/keys/id_github.age` | |
| 2. Deployed to `~/.ssh/id_ed25519_github` | |
| 3. Loaded into ssh-agent via keychain (if no passphrase) | |
🤖 Prompt for AI Agents
In named-hosts/kyber/README.md around lines 83 to 88, the documentation
references the outdated filename id_ed25519.age; update that line to the actual
secret filename id_github.age so the steps reflect the secrets configuration
(i.e., decrypt from named-hosts/galactica/keys/id_github.age, then deploy/load
as described).
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.
The secret file for the GitHub SSH key was renamed from id_ed25519.age to id_github.age in the Nix configurations, but this documentation still refers to the old name. Please update it to maintain consistency and avoid confusion.
| named-hosts/galactica/keys/id_ed25519.age | |
| named-hosts/galactica/keys/id_github.age |
Copilot
AI
Dec 13, 2025
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.
The documentation references id_ed25519.age but the code has been updated to use id_github.age. This inconsistency will confuse users trying to follow the troubleshooting instructions. Update this line to reference id_github.age to match the code changes.
| named-hosts/galactica/keys/id_ed25519.age | |
| named-hosts/galactica/keys/id_github.age |
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.
Troubleshooting command uses wrong filename.
The manual decrypt command references id_ed25519.age instead of id_github.age.
# Manually deploy if needed
age -d -i ~/.ssh/id_ed25519 -o ~/.ssh/id_ed25519_github \
- named-hosts/galactica/keys/id_ed25519.age
+ named-hosts/galactica/keys/id_github.age
chmod 0600 ~/.ssh/id_ed25519_github🤖 Prompt for AI Agents
In named-hosts/kyber/README.md around lines 115 to 118, the manual decrypt
command references the wrong filename (id_ed25519.age) — it should reference
id_github.age; update the age command to use the correct encrypted file path
(named-hosts/galactica/keys/id_github.age) so the correct key is decrypted, and
keep the subsequent chmod step unchanged.
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.
The script uses bash to evaluate keychain output but then immediately checks if ssh-add works, which may not reflect the environment set in the bash subprocess. The keychain evaluation in bash won't affect the Fish shell's environment. Consider using Fish's native evaluation of keychain output instead.