diff --git a/apps/api/src/api/tool_instances.py b/apps/api/src/api/tool_instances.py index e718354..b41b517 100644 --- a/apps/api/src/api/tool_instances.py +++ b/apps/api/src/api/tool_instances.py @@ -1656,7 +1656,9 @@ async def start_instance( container_user = ( "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") logger.debug( diff --git a/apps/api/src/services/permission_fixer.py b/apps/api/src/services/permission_fixer.py index 3161f1b..f2db7cc 100644 --- a/apps/api/src/services/permission_fixer.py +++ b/apps/api/src/services/permission_fixer.py @@ -104,6 +104,44 @@ def apply_mount_permissions( 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( container_id: str, ssh_target: str, @@ -127,20 +165,24 @@ def apply_ssh_permissions( """ result: dict[str, Any] = {"success": True, "error": None} try: - # Ensure directory is owned by the container user - _run_in_container( + # 1. Ensure directory is owned by the container user + _exec_and_log( container_id, ["chown", "-R", f"{container_user}:{container_user}", ssh_target], timeout, + "chown", ) - # Set directory permissions - _run_in_container( + + # 2. Set directory permissions + _exec_and_log( container_id, ["chmod", "700", ssh_target], 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, [ "sh", @@ -148,12 +190,38 @@ def apply_ssh_permissions( f"find {ssh_target} -name 'id_*' -type f -exec chmod 600 {{}} +", ], 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, ssh_target, - container_id, + ls_output, + stat_output, + key_stat, ) except PermissionFixError as exc: result["success"] = False diff --git a/apps/api/tests/unit/test_permission_fixer.py b/apps/api/tests/unit/test_permission_fixer.py index e863eb1..8b09302 100644 --- a/apps/api/tests/unit/test_permission_fixer.py +++ b/apps/api/tests/unit/test_permission_fixer.py @@ -136,32 +136,35 @@ class TestRunInContainer: class TestApplySshPermissions: """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: + mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="") result = apply_ssh_permissions("abc123", "/home/user/.ssh", "user") assert result["success"] is True - assert mock_run.call_count == 3 - chown_call = mock_run.call_args_list[0] - chmod_call = mock_run.call_args_list[1] - file_mode_call = mock_run.call_args_list[2] + # 3 fix commands + 3 verification commands + assert mock_run.call_count == 6 + chown_cmd = mock_run.call_args_list[0][0][0] + 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 chmod_call[0][1] == ["chmod", "700", "/home/user/.ssh"] - assert file_mode_call[0][1][0] == "sh" - assert "find /home/user/.ssh -name 'id_*' -type f -exec chmod 600" in file_mode_call[0][1][2] + assert chown_cmd == ["docker", "exec", "--user", "root", "abc123", "chown", "-R", "user:user", "/home/user/.ssh"] + assert chmod_cmd == ["docker", "exec", "--user", "root", "abc123", "chmod", "700", "/home/user/.ssh"] + assert file_mode_cmd[0] == "docker" + 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: + mock_run.return_value = MagicMock(returncode=0, stdout="", stderr="") result = apply_ssh_permissions("abc123", "/root/.ssh", "root") assert result["success"] is True - chown_call = mock_run.call_args_list[0] - assert chown_call[0][1] == ["chown", "-R", "root:root", "/root/.ssh"] + chown_cmd = mock_run.call_args_list[0][0][0] + 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: - 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")