Skip to content

Commit 81374af

Browse files
jsbattigclaude
andcommitted
fix: SSH Keys UI - remove migration banner and compute unmanaged key fingerprints
- Remove useless "Migration Status" section from ssh_keys.html template - Add fingerprint computation to KeyDiscoveryService using ssh-keygen -lf - Unmanaged keys now show SHA256 fingerprints instead of "unknown" - Graceful error handling if ssh-keygen fails for any key 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent f904b37 commit 81374af

File tree

4 files changed

+109
-33
lines changed

4 files changed

+109
-33
lines changed

src/code_indexer/server/services/key_discovery_service.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
Discovers existing SSH keys and parses config file mappings.
55
"""
66

7+
import subprocess
78
from dataclasses import dataclass
89
from pathlib import Path
910
from typing import Dict, List, Optional
@@ -44,6 +45,33 @@ def __init__(self, ssh_dir: Optional[Path] = None):
4445
ssh_dir = Path.home() / ".ssh"
4546
self.ssh_dir = ssh_dir
4647

48+
def _compute_fingerprint(self, public_key_path: Path) -> Optional[str]:
49+
"""
50+
Compute fingerprint of an SSH public key using ssh-keygen.
51+
52+
Args:
53+
public_key_path: Path to the public key file
54+
55+
Returns:
56+
Fingerprint string (e.g., "SHA256:xxxx...") or None if computation fails
57+
"""
58+
try:
59+
result = subprocess.run(
60+
["ssh-keygen", "-lf", str(public_key_path)],
61+
capture_output=True,
62+
text=True,
63+
timeout=5,
64+
)
65+
if result.returncode == 0 and result.stdout:
66+
# Output format: "256 SHA256:xxxx... user@host (ED25519)"
67+
# Extract the SHA256:xxxx... part (second field)
68+
parts = result.stdout.strip().split()
69+
if len(parts) >= 2:
70+
return parts[1]
71+
return None
72+
except (subprocess.CalledProcessError, subprocess.TimeoutExpired, OSError):
73+
return None
74+
4775
def discover_existing_keys(self) -> List[KeyInfo]:
4876
"""
4977
Discover existing SSH key pairs in the SSH directory.
@@ -73,11 +101,13 @@ def discover_existing_keys(self) -> List[KeyInfo]:
73101
# Check if corresponding .pub file exists
74102
pub_path = file_path.parent / f"{file_path.name}.pub"
75103
if pub_path.exists():
104+
fingerprint = self._compute_fingerprint(pub_path)
76105
discovered_keys.append(
77106
KeyInfo(
78107
name=file_path.name,
79108
private_path=file_path,
80109
public_path=pub_path,
110+
fingerprint=fingerprint,
81111
is_cidx_managed=False,
82112
)
83113
)

src/code_indexer/server/web/templates/ssh_keys.html

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -91,34 +91,6 @@ <h2>Create New SSH Key</h2>
9191
</article>
9292
</section>
9393

94-
<!-- Migration Status Section -->
95-
{% if migration_result %}
96-
<article>
97-
<header>
98-
<h2>Migration Status</h2>
99-
</header>
100-
{% if migration_result.skipped %}
101-
<p><mark>Migration already completed</mark></p>
102-
{% else %}
103-
<p>
104-
<strong>{{ migration_result.keys_discovered }}</strong> keys discovered,
105-
<strong>{{ migration_result.keys_imported }}</strong> imported,
106-
<strong>{{ migration_result.mappings_imported }}</strong> existing mappings imported
107-
</p>
108-
{% if migration_result.failed_hosts %}
109-
<details>
110-
<summary>Failed Hosts ({{ migration_result.failed_hosts|length }})</summary>
111-
<ul>
112-
{% for key_name, hostname, reason in migration_result.failed_hosts %}
113-
<li>{{ key_name }} -> {{ hostname }}: {{ reason }}</li>
114-
{% endfor %}
115-
</ul>
116-
</details>
117-
{% endif %}
118-
{% endif %}
119-
</article>
120-
{% endif %}
121-
12294
<!-- Managed Keys Section -->
12395
<article>
12496
<header>

tests/server/services/test_key_discovery_service.py

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
"""Unit tests for KeyDiscoveryService."""
22

3+
import subprocess
34
from pathlib import Path
5+
from unittest.mock import patch, MagicMock
46

57
from code_indexer.server.services.key_discovery_service import (
68
KeyDiscoveryService,
@@ -123,3 +125,73 @@ def test_parse_mappings_extracts_identity_files(self, tmp_path):
123125

124126
assert work_key_path in mappings
125127
assert "gitlab.com" in mappings[work_key_path]
128+
129+
130+
class TestKeyDiscoveryServiceFingerprint:
131+
"""Tests for fingerprint computation in discover_existing_keys()."""
132+
133+
def test_discover_keys_computes_fingerprint(self, tmp_path):
134+
"""Should compute fingerprint for discovered keys."""
135+
ssh_dir = tmp_path / ".ssh"
136+
ssh_dir.mkdir()
137+
138+
# Create a key pair
139+
(ssh_dir / "id_ed25519").write_text("PRIVATE KEY CONTENT")
140+
(ssh_dir / "id_ed25519.pub").write_text("ssh-ed25519 AAAA... test@example.com")
141+
142+
service = KeyDiscoveryService(ssh_dir=ssh_dir)
143+
144+
# Mock _compute_fingerprint to return a known fingerprint
145+
with patch.object(
146+
service, "_compute_fingerprint", return_value="SHA256:abcdef123456"
147+
) as mock_compute:
148+
keys = service.discover_existing_keys()
149+
150+
# Verify _compute_fingerprint was called with the public key path
151+
mock_compute.assert_called_once()
152+
call_args = mock_compute.call_args
153+
assert call_args[0][0] == ssh_dir / "id_ed25519.pub"
154+
155+
assert len(keys) == 1
156+
assert keys[0].fingerprint == "SHA256:abcdef123456"
157+
158+
def test_discover_keys_handles_fingerprint_failure_gracefully(self, tmp_path):
159+
"""Should leave fingerprint as None if computation fails."""
160+
ssh_dir = tmp_path / ".ssh"
161+
ssh_dir.mkdir()
162+
163+
# Create a key pair
164+
(ssh_dir / "id_rsa").write_text("PRIVATE KEY CONTENT")
165+
(ssh_dir / "id_rsa.pub").write_text("ssh-rsa AAAA... test@example.com")
166+
167+
service = KeyDiscoveryService(ssh_dir=ssh_dir)
168+
169+
# Mock _compute_fingerprint to return None (failure case)
170+
with patch.object(service, "_compute_fingerprint", return_value=None):
171+
keys = service.discover_existing_keys()
172+
173+
assert len(keys) == 1
174+
assert keys[0].fingerprint is None
175+
176+
def test_compute_fingerprint_parses_ssh_keygen_output(self, tmp_path):
177+
"""Should correctly parse fingerprint from ssh-keygen output."""
178+
import code_indexer.server.services.key_discovery_service as kds_module
179+
180+
ssh_dir = tmp_path / ".ssh"
181+
ssh_dir.mkdir()
182+
183+
# Create a valid public key file
184+
pub_key_path = ssh_dir / "id_ed25519.pub"
185+
pub_key_path.write_text("ssh-ed25519 AAAA...")
186+
187+
service = KeyDiscoveryService(ssh_dir=ssh_dir)
188+
189+
# Mock subprocess.run at the module level where it's imported
190+
mock_result = MagicMock()
191+
mock_result.stdout = "3072 SHA256:xyzPQR789abc user@host (RSA)\n"
192+
mock_result.returncode = 0
193+
194+
with patch.object(kds_module.subprocess, "run", return_value=mock_result):
195+
fingerprint = service._compute_fingerprint(pub_key_path)
196+
197+
assert fingerprint == "SHA256:xyzPQR789abc"

tests/server/web/test_ssh_keys_web_ui.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ def test_ssh_keys_template_exists(self):
1919
)
2020
assert template_path.exists(), f"Template not found at {template_path}"
2121

22-
def test_ssh_keys_template_contains_migration_status(self):
23-
"""Template should contain migration status display elements."""
22+
def test_ssh_keys_template_no_migration_status_section(self):
23+
"""Template should NOT contain migration status section (removed as useless)."""
2424
template_path = (
2525
Path(__file__).parent.parent.parent.parent
2626
/ "src"
@@ -33,11 +33,13 @@ def test_ssh_keys_template_contains_migration_status(self):
3333

3434
content = template_path.read_text()
3535

36-
# Should have migration summary section
37-
assert "migration" in content.lower()
36+
# Migration status section should be removed
37+
assert "migration_result" not in content
38+
assert "Migration Status" not in content
3839

39-
# Should have key listing
40+
# Should still have key listing
4041
assert "key" in content.lower()
42+
assert "managed" in content.lower()
4143

4244
def test_ssh_keys_template_has_copy_button(self):
4345
"""Template should have copy public key functionality."""

0 commit comments

Comments
 (0)