Skip to content

Commit 47db6b5

Browse files
adamamylclaude
andcommitted
fix(pseudohome): fix .ssh perm clobbering, prompt on SSH failure
- clone_or_update_repo: move group chgrp/chmod from parent_dir to dest_dir post-clone; prevents recursive clobber of .ssh perms - set_ssh_perms: chmod 600 all .ssh/* before relaxing pub/known_hosts to 644, so authorized_keys and private keys always land at 600 - probe_and_fix_ssh: return bool instead of raising on network failure, letting the caller prompt rather than abort - module_pseudohome: prompt to add deploy key when probe returns False; wrap clone in try/finally so set_ssh_perms always runs even on failure - clone_or_update_private_repo_with_key_check: expand is_ssh_error to cover timeout/network failures (not just "Permission denied"), so the interactive key prompt fires on connection timeout too Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01P4Hxvsa529m9F6d6dxLCiG
1 parent 484bf41 commit 47db6b5

3 files changed

Lines changed: 62 additions & 49 deletions

File tree

lib/installer_utils/git_tools.py

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -31,16 +31,9 @@ def clone_or_update_repo(exec_obj: Executor,
3131
"""
3232

3333
parent_dir = os.path.dirname(dest_dir)
34-
35-
# 1. Ensure parent dir exists and has correct group/permissions
34+
35+
# 1. Ensure parent dir exists
3636
exec_obj.run(f"mkdir -p {parent_dir}", force_sudo=True)
37-
38-
# Set group ownership and ensure group-writeable; only when a group is explicitly requested.
39-
# Never apply to home directories — callers that need a specific group (e.g. docker) pass it.
40-
if group:
41-
exec_obj.run(f"chgrp -R {group} {parent_dir} || true", force_sudo=True)
42-
exec_obj.run(f"chmod -R g+w {parent_dir}", force_sudo=True)
43-
exec_obj.run(f"chmod -R -s {parent_dir} || true", force_sudo=True)
4437

4538
# 2. Prepare environment prefix for SSH key usage
4639
env_prefix = ""
@@ -89,6 +82,12 @@ def clone_or_update_repo(exec_obj: Executor,
8982
exec_obj.run(final_cmd, user=user)
9083
log.success(f"Repository cloned: {dest_dir}")
9184

85+
# Apply group ownership to the repo dir only (not the parent — avoids clobbering .ssh etc.)
86+
if group and os.path.isdir(dest_dir):
87+
exec_obj.run(f"chgrp -R {group} {dest_dir} || true", force_sudo=True)
88+
exec_obj.run(f"chmod -R g+w {dest_dir}", force_sudo=True)
89+
exec_obj.run(f"chmod -R -s {dest_dir} || true", force_sudo=True)
90+
9291
def clone_or_update_private_repo_with_key_check(exec_obj: Executor,
9392
repo_url: str,
9493
dest_dir: str,
@@ -124,9 +123,20 @@ def clone_or_update_private_repo_with_key_check(exec_obj: Executor,
124123
break # Exit loop on success
125124

126125
except subprocess.CalledProcessError as e:
127-
# Check for typical Git/SSH permission error (Exit code 128)
128-
is_ssh_error = (e.returncode == 128) and ("Permission denied" in e.stderr)
129-
126+
# Treat any git/SSH failure (auth or network) as potentially fixable by adding the key.
127+
# Timeout ("Connection timed out") and auth ("Permission denied") both exit 128.
128+
is_ssh_error = e.returncode == 128 and any(
129+
marker in (e.stderr or "")
130+
for marker in [
131+
"Permission denied",
132+
"Connection timed out",
133+
"connect to host",
134+
"Host is unreachable",
135+
"No route to host",
136+
"Network is unreachable",
137+
]
138+
)
139+
130140
if attempt == 0 and is_ssh_error:
131141
log.warning("Initial clone attempt failed due to possible missing deploy key.")
132142

@@ -213,9 +223,10 @@ def set_ssh_perms(exec_obj: Executor, user: str, ssh_dir: str) -> None:
213223
exec_obj.run(f"chmod 700 {ssh_dir}", force_sudo=True)
214224
exec_obj.run(f"chown {user}:{user} {ssh_dir}", force_sudo=True)
215225

216-
# Re-fix ownership in case root generated the keys; _create_if_needed_ssh_key handles 600/644.
226+
# Re-fix ownership and enforce 600 on all files; public keys and known_hosts relaxed below.
217227
exec_obj.run(f"chown {user}:{user} {ssh_dir}/* || true", force_sudo=True)
218-
219-
# We explicitly relax permissions on known_hosts and public keys to 644/400, just in case
228+
exec_obj.run(f"chmod 600 {ssh_dir}/* || true", force_sudo=True)
229+
230+
# Relax known_hosts and public keys to 644
220231
exec_obj.run(f"chmod 644 {ssh_dir}/known_hosts || true", force_sudo=True)
221232
exec_obj.run(f"chmod 644 {ssh_dir}/*.pub || true", force_sudo=True)

lib/installer_utils/module_pseudohome.py

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
set_homedir_perms_recursively,
88
set_ssh_perms,
99
)
10-
from .repo_utils import _create_if_needed_ssh_key
10+
from .repo_utils import _create_if_needed_ssh_key, _display_key_and_url_for_repo
1111
from .ssh_utils import probe_and_fix_ssh
1212
from .tailscale import ensure_tailscale_connected
1313

@@ -52,30 +52,36 @@ def setup_pseudohome(exec_obj: Executor) -> None:
5252
# 4. SSH connectivity probe — remediates known_hosts / key perms, prompts if key missing
5353
ssh_key_path = os.path.join(ssh_dir, repo_name)
5454
log.info("Probing SSH connectivity to git.amyl.org.uk...")
55-
probe_and_fix_ssh(
55+
ssh_ok = probe_and_fix_ssh(
5656
exec_obj,
5757
host="git.amyl.org.uk",
5858
ssh_user=user,
5959
key_path=ssh_key_path,
6060
)
61+
if not ssh_ok and not exec_obj.force:
62+
log.warning(
63+
"Cannot reach git.amyl.org.uk — the pseudohome deploy key may not be authorised yet."
64+
)
65+
_display_key_and_url_for_repo(exec_obj, ssh_dir, repo_name, PSEUDOHOME_REPO_URL)
6166

62-
# 5. Clone/Update Repo (Handles interactive key prompt and retry on failure)
63-
64-
clone_or_update_private_repo_with_key_check(
65-
exec_obj,
66-
PSEUDOHOME_REPO_URL,
67-
dest_dir,
68-
ssh_key_path=ssh_key_path,
69-
repo_name=repo_name,
70-
extra_git_flags="--recursive",
71-
user=user, # Execute as 'adam'
72-
)
67+
# 5. Clone/Update Repo — always re-enforce .ssh perms even if clone fails.
68+
try:
69+
clone_or_update_private_repo_with_key_check(
70+
exec_obj,
71+
PSEUDOHOME_REPO_URL,
72+
dest_dir,
73+
ssh_key_path=ssh_key_path,
74+
repo_name=repo_name,
75+
extra_git_flags="--recursive",
76+
user=user,
77+
)
7378

74-
# 6. Fix permissions: home dir ownership (non-recursive — must not clobber .ssh),
75-
# then repo contents, then re-enforce .ssh in case anything above touched it.
76-
exec_obj.run(f"chown {user}:{user} {os.path.dirname(dest_dir)}", force_sudo=True)
77-
set_homedir_perms_recursively(exec_obj, user, dest_dir)
78-
set_ssh_perms(exec_obj, user, ssh_dir)
79+
# 6. Fix permissions on repo dir; home dir chown is non-recursive.
80+
exec_obj.run(f"chown {user}:{user} {os.path.dirname(dest_dir)}", force_sudo=True)
81+
set_homedir_perms_recursively(exec_obj, user, dest_dir)
82+
finally:
83+
# Always re-enforce .ssh regardless of clone outcome — group chmod can clobber these.
84+
set_ssh_perms(exec_obj, user, ssh_dir)
7985

8086
# 7. Run installer script (as the user)
8187
installer_path = os.path.join(dest_dir, PSEUDOHOME_INSTALLER)

lib/installer_utils/ssh_utils.py

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -82,17 +82,18 @@ def probe_and_fix_ssh(
8282
host: str,
8383
ssh_user: str,
8484
key_path: str,
85-
) -> None:
85+
) -> bool:
8686
"""
8787
Tests SSH connectivity to host as ssh_user using key_path.
88-
Auto-remediates where possible, raises RuntimeError if unresolvable.
88+
Auto-remediates where possible (known_hosts, key perms).
89+
Returns True if SSH is working, False if not (caller should prompt user to add the key).
8990
9091
Remediations attempted (in order):
91-
1. Immediate raise on network-level failure (Tailscale/routing issue)
92+
1. Return False on network-level failure (caller will prompt)
9293
2. ssh-keygen -R to clear any stale host key entry
9394
3. ssh-keyscan to populate/refresh known_hosts
9495
4. chmod 700 on .ssh dir and 600 on private key if permissions wrong
95-
5. Verbose retry with full diagnostics before giving up
96+
5. Verbose retry with full diagnostics before returning False
9697
"""
9798
_validate_host(host)
9899
home = _user_homedir(ssh_user)
@@ -101,7 +102,7 @@ def probe_and_fix_ssh(
101102
ok, stderr = _ssh_probe(exec_obj, host, ssh_user, key_path)
102103
if ok:
103104
log.success(f"SSH to {ssh_user}@{host}: OK")
104-
return
105+
return True
105106

106107
log.warning(f"SSH to {host} failed — diagnosing...")
107108

@@ -116,15 +117,13 @@ def probe_and_fix_ssh(
116117
]
117118
)
118119
if is_network_fail:
119-
raise RuntimeError(
120-
f"Network failure connecting to {host}Tailscale connected but host unreachable?\n"
120+
log.warning(
121+
f"Network failure connecting to {host}key may not be authorised yet.\n"
121122
f"SSH said: {stderr.strip()}"
122123
)
124+
return False
123125

124126
# Remediation A: clear stale host key unconditionally before re-scanning.
125-
# ssh-keygen -R is harmless when no entry exists; and -q suppresses the
126-
# "REMOTE HOST IDENTIFICATION HAS CHANGED" warning so we can't detect it
127-
# from the quiet probe — easier to always clean and re-add.
128127
log.info(f"Clearing any stale known_hosts entry for {host}...")
129128
exec_obj.run(["ssh-keygen", "-R", host], user=ssh_user, check=False, run_quiet=True)
130129

@@ -149,15 +148,12 @@ def probe_and_fix_ssh(
149148
ok, stderr = _ssh_probe(exec_obj, host, ssh_user, key_path)
150149
if ok:
151150
log.success(f"SSH to {ssh_user}@{host}: OK (after remediation)")
152-
return
151+
return True
153152

154-
# Verbose diagnostics before giving up
153+
# Verbose diagnostics
155154
log.error(f"SSH to {host} still failing. Verbose output:")
156155
_, verbose_stderr = _ssh_probe(exec_obj, host, ssh_user, key_path, verbose=True)
157156
for line in verbose_stderr.splitlines():
158157
log.error(f" ssh: {line}")
159158

160-
raise RuntimeError(
161-
f"Cannot SSH to {ssh_user}@{host} — ensure the public key is authorised on the remote.\n"
162-
f"Public key to add: {key_path}.pub"
163-
)
159+
return False

0 commit comments

Comments
 (0)