fix: add detailed SSH permission fix logging for debugging
- Replace apply_ssh_permissions internals with _exec_and_log for full visibility - Log every docker exec command, stdout, and stderr at DEBUG level - After chown/chmod, run ls -la and stat to verify final state - Log verified state at INFO level so users can see exactly what happened - Update tests to mock subprocess.run instead of _run_in_container Quality gates: pytest 236 passed (6 pre-existing), tsc --noEmit clean
This commit is contained in:
@@ -1656,7 +1656,9 @@ async def start_instance(
|
|||||||
container_user = (
|
container_user = (
|
||||||
"root"
|
"root"
|
||||||
if home_dir == "/root"
|
if home_dir == "/root"
|
||||||
else home_dir[6:] if home_dir.startswith("/home/") else "root"
|
else home_dir[6:]
|
||||||
|
if home_dir.startswith("/home/")
|
||||||
|
else "root"
|
||||||
)
|
)
|
||||||
ssh_target = os.path.join(home_dir, ".ssh")
|
ssh_target = os.path.join(home_dir, ".ssh")
|
||||||
logger.debug(
|
logger.debug(
|
||||||
|
|||||||
@@ -104,6 +104,44 @@ def apply_mount_permissions(
|
|||||||
return results
|
return results
|
||||||
|
|
||||||
|
|
||||||
|
def _exec_and_log(
|
||||||
|
container_id: str,
|
||||||
|
command: list[str],
|
||||||
|
timeout: int,
|
||||||
|
description: str,
|
||||||
|
) -> str:
|
||||||
|
"""Run a docker exec command and log stdout/stderr for debugging."""
|
||||||
|
cmd = ["docker", "exec", "--user", "root", container_id] + command
|
||||||
|
logger.debug("[SSH-fix] %s: %s", description, " ".join(cmd))
|
||||||
|
|
||||||
|
try:
|
||||||
|
result = subprocess.run(
|
||||||
|
cmd,
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
timeout=timeout,
|
||||||
|
)
|
||||||
|
except subprocess.TimeoutExpired:
|
||||||
|
raise PermissionFixError(
|
||||||
|
f"Command timed out after {timeout}s: {' '.join(command)}"
|
||||||
|
)
|
||||||
|
except FileNotFoundError:
|
||||||
|
raise PermissionFixError(f"Docker command not found: {' '.join(command)}")
|
||||||
|
|
||||||
|
stdout = result.stdout.strip()
|
||||||
|
stderr = result.stderr.strip()
|
||||||
|
if stdout:
|
||||||
|
logger.debug("[SSH-fix] %s stdout: %s", description, stdout)
|
||||||
|
if stderr:
|
||||||
|
logger.debug("[SSH-fix] %s stderr: %s", description, stderr)
|
||||||
|
|
||||||
|
if result.returncode != 0:
|
||||||
|
raise PermissionFixError(
|
||||||
|
f"Command failed (rc={result.returncode}): {stderr or '(no stderr)'}"
|
||||||
|
)
|
||||||
|
return stdout
|
||||||
|
|
||||||
|
|
||||||
def apply_ssh_permissions(
|
def apply_ssh_permissions(
|
||||||
container_id: str,
|
container_id: str,
|
||||||
ssh_target: str,
|
ssh_target: str,
|
||||||
@@ -127,20 +165,24 @@ def apply_ssh_permissions(
|
|||||||
"""
|
"""
|
||||||
result: dict[str, Any] = {"success": True, "error": None}
|
result: dict[str, Any] = {"success": True, "error": None}
|
||||||
try:
|
try:
|
||||||
# Ensure directory is owned by the container user
|
# 1. Ensure directory is owned by the container user
|
||||||
_run_in_container(
|
_exec_and_log(
|
||||||
container_id,
|
container_id,
|
||||||
["chown", "-R", f"{container_user}:{container_user}", ssh_target],
|
["chown", "-R", f"{container_user}:{container_user}", ssh_target],
|
||||||
timeout,
|
timeout,
|
||||||
|
"chown",
|
||||||
)
|
)
|
||||||
# Set directory permissions
|
|
||||||
_run_in_container(
|
# 2. Set directory permissions
|
||||||
|
_exec_and_log(
|
||||||
container_id,
|
container_id,
|
||||||
["chmod", "700", ssh_target],
|
["chmod", "700", ssh_target],
|
||||||
timeout,
|
timeout,
|
||||||
|
"chmod-dir",
|
||||||
)
|
)
|
||||||
# Set private key permissions
|
|
||||||
_run_in_container(
|
# 3. Set private key permissions (id_ed25519, id_rsa, etc.)
|
||||||
|
_exec_and_log(
|
||||||
container_id,
|
container_id,
|
||||||
[
|
[
|
||||||
"sh",
|
"sh",
|
||||||
@@ -148,12 +190,38 @@ def apply_ssh_permissions(
|
|||||||
f"find {ssh_target} -name 'id_*' -type f -exec chmod 600 {{}} +",
|
f"find {ssh_target} -name 'id_*' -type f -exec chmod 600 {{}} +",
|
||||||
],
|
],
|
||||||
timeout,
|
timeout,
|
||||||
|
"chmod-keys",
|
||||||
)
|
)
|
||||||
logger.debug(
|
|
||||||
"Applied SSH permissions for user %s on %s in container %s",
|
# 4. Verify final state
|
||||||
|
ls_output = _exec_and_log(
|
||||||
|
container_id,
|
||||||
|
["ls", "-la", ssh_target],
|
||||||
|
timeout,
|
||||||
|
"verify-ls",
|
||||||
|
)
|
||||||
|
stat_output = _exec_and_log(
|
||||||
|
container_id,
|
||||||
|
["stat", "-c", "%U:%G %a %n", ssh_target],
|
||||||
|
timeout,
|
||||||
|
"verify-stat-dir",
|
||||||
|
)
|
||||||
|
key_stat = _exec_and_log(
|
||||||
|
container_id,
|
||||||
|
["sh", "-c", f"stat -c '%U:%G %a %n' {ssh_target}/id_* 2>/dev/null || echo 'no id_* files found'"],
|
||||||
|
timeout,
|
||||||
|
"verify-stat-keys",
|
||||||
|
)
|
||||||
|
|
||||||
|
logger.info(
|
||||||
|
"SSH permissions fixed for container %s (user=%s, target=%s). "
|
||||||
|
"ls:\n%s\nstat-dir: %s\nstat-keys: %s",
|
||||||
|
container_id,
|
||||||
container_user,
|
container_user,
|
||||||
ssh_target,
|
ssh_target,
|
||||||
container_id,
|
ls_output,
|
||||||
|
stat_output,
|
||||||
|
key_stat,
|
||||||
)
|
)
|
||||||
except PermissionFixError as exc:
|
except PermissionFixError as exc:
|
||||||
result["success"] = False
|
result["success"] = False
|
||||||
|
|||||||
@@ -136,32 +136,35 @@ class TestRunInContainer:
|
|||||||
class TestApplySshPermissions:
|
class TestApplySshPermissions:
|
||||||
"""Tests for apply_ssh_permissions."""
|
"""Tests for apply_ssh_permissions."""
|
||||||
|
|
||||||
@patch("src.services.permission_fixer._run_in_container")
|
@patch("subprocess.run")
|
||||||
def test_applies_chown_chmod_and_file_mode(self, mock_run) -> None:
|
def test_applies_chown_chmod_and_file_mode(self, mock_run) -> None:
|
||||||
|
mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="")
|
||||||
result = apply_ssh_permissions("abc123", "/home/user/.ssh", "user")
|
result = apply_ssh_permissions("abc123", "/home/user/.ssh", "user")
|
||||||
|
|
||||||
assert result["success"] is True
|
assert result["success"] is True
|
||||||
assert mock_run.call_count == 3
|
# 3 fix commands + 3 verification commands
|
||||||
chown_call = mock_run.call_args_list[0]
|
assert mock_run.call_count == 6
|
||||||
chmod_call = mock_run.call_args_list[1]
|
chown_cmd = mock_run.call_args_list[0][0][0]
|
||||||
file_mode_call = mock_run.call_args_list[2]
|
chmod_cmd = mock_run.call_args_list[1][0][0]
|
||||||
|
file_mode_cmd = mock_run.call_args_list[2][0][0]
|
||||||
|
|
||||||
assert chown_call[0][1] == ["chown", "-R", "user:user", "/home/user/.ssh"]
|
assert chown_cmd == ["docker", "exec", "--user", "root", "abc123", "chown", "-R", "user:user", "/home/user/.ssh"]
|
||||||
assert chmod_call[0][1] == ["chmod", "700", "/home/user/.ssh"]
|
assert chmod_cmd == ["docker", "exec", "--user", "root", "abc123", "chmod", "700", "/home/user/.ssh"]
|
||||||
assert file_mode_call[0][1][0] == "sh"
|
assert file_mode_cmd[0] == "docker"
|
||||||
assert "find /home/user/.ssh -name 'id_*' -type f -exec chmod 600" in file_mode_call[0][1][2]
|
assert "find /home/user/.ssh -name 'id_*' -type f -exec chmod 600" in file_mode_cmd[-1]
|
||||||
|
|
||||||
@patch("src.services.permission_fixer._run_in_container")
|
@patch("subprocess.run")
|
||||||
def test_uses_root_user(self, mock_run) -> None:
|
def test_uses_root_user(self, mock_run) -> None:
|
||||||
|
mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="")
|
||||||
result = apply_ssh_permissions("abc123", "/root/.ssh", "root")
|
result = apply_ssh_permissions("abc123", "/root/.ssh", "root")
|
||||||
|
|
||||||
assert result["success"] is True
|
assert result["success"] is True
|
||||||
chown_call = mock_run.call_args_list[0]
|
chown_cmd = mock_run.call_args_list[0][0][0]
|
||||||
assert chown_call[0][1] == ["chown", "-R", "root:root", "/root/.ssh"]
|
assert chown_cmd == ["docker", "exec", "--user", "root", "abc123", "chown", "-R", "root:root", "/root/.ssh"]
|
||||||
|
|
||||||
@patch("src.services.permission_fixer._run_in_container")
|
@patch("subprocess.run")
|
||||||
def test_reports_failure(self, mock_run) -> None:
|
def test_reports_failure(self, mock_run) -> None:
|
||||||
mock_run.side_effect = PermissionFixError("chown failed")
|
mock_run.return_value = MagicMock(returncode=1, stdout="", stderr="chown failed")
|
||||||
|
|
||||||
result = apply_ssh_permissions("abc123", "/home/user/.ssh", "user")
|
result = apply_ssh_permissions("abc123", "/home/user/.ssh", "user")
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user