Add SSHWrapper driver #647

coderabbitai · 2025-09-29T09:08:51Z

⚠️ Potential issue | 🟠 Major

Ensure wrapper-enforced port cannot be overridden by user “-p”

If the wrapper supplies a port (direct or via forward), a user “-p …” in args currently wins because it’s appended later. Normalize options so the enforced port is last and unique.

Apply this diff to move port handling into the final assembly and drop user-provided “-p” variants before appending the enforced port:

@@ def _run_ssh_local(self, host, port, ssh_command, default_username, args): - # Build final SSH command - ssh_args = self._build_final_ssh_command(ssh_args, ssh_options, host, command_args) + # Build final SSH command + ssh_args = self._build_final_ssh_command(ssh_args, ssh_options, host, port, command_args) @@ - def _build_final_ssh_command(self, ssh_args, ssh_options, host, command_args): + def _build_final_ssh_command(self, ssh_args, ssh_options, host, port, command_args): """Build the final SSH command with all components""" - # Add SSH options - ssh_args.extend(ssh_options) + # Normalize port (-p) so the wrapper-provided port (if any) wins + ssh_args.extend(self._normalize_port_option(ssh_options, port)) @@ # Add hostname before command arguments if host: ssh_args.append(host) @@ self.logger.debug(f"Running SSH command: {ssh_args}") return ssh_args + + def _normalize_port_option(self, ssh_options, port): + """Remove user -p overrides and append enforced port last (if provided).""" + normalized: list[str] = [] + i = 0 + while i < len(ssh_options): + tok = ssh_options[i] + # Drop -p VALUE + if tok == "-p": + i += 2 if i + 1 < len(ssh_options) else 1 + continue + # Drop inline -pVALUE + if tok.startswith("-p") and tok != "-p": + i += 1 + continue + normalized.append(tok) + i += 1 + if port and port != 22: + normalized.extend(["-p", str(port)]) + return normalized

Also applies to: 159-173

🤖 Prompt for AI Agents

packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py around lines 79-86 (also applies to 159-173): user-supplied “-p”/“--port” currently wins because it’s appended after ssh_options; remove any user-provided port flags from the combined args during final assembly, then append the wrapper-enforced port option last (ensuring only one port flag exists) — move port handling into _build_final_ssh_command so it normalizes duplicates (strip "-p" and "--port" and their values from args/ssh_options), and finally append the enforced port as the last unique option before executing.

coderabbitai · 2025-09-29T09:08:51Z

⚠️ Potential issue | 🟠 Major

Detect username via “-luser” and “-o User=…” too; don’t miss inline forms

Only checking for a standalone “-l” misses common forms (“-lroot”, “-oUser=alice”). That can incorrectly inject a default username.

Apply this diff to improve detection and avoid adding a default user when user is already specified:

@@ def _build_ssh_command_args(self, ssh_command, port, default_username, args): - # Add port if specified - if port and port != 22: - ssh_args.extend(["-p", str(port)]) - # Check if user already provided a username with -l flag in SSH options only # We need to separate SSH options from command args first to avoid false positives ssh_options, _ = self._separate_ssh_options_and_command_args(args) - has_user_flag = any( - ssh_options[i] == "-l" and i + 1 < len(ssh_options) - for i in range(len(ssh_options)) - ) + has_user_flag = False + i = 0 + while i < len(ssh_options): + tok = ssh_options[i] + # -l user or inline -luser + if tok == "-l" and i + 1 < len(ssh_options): + has_user_flag = True + break + if tok.startswith("-l") and tok != "-l": + has_user_flag = True + break + # -o User=alice or inline -oUser=alice + if tok == "-o" and i + 1 < len(ssh_options): + val = str(ssh_options[i + 1]).lower() + if val.startswith("user=") or val.startswith("user "): + has_user_flag = True + break + i += 1 # skip value + elif tok.startswith("-o") and len(tok) > 2: + oval = tok[2:].lower() + if oval.startswith("user=") or oval.startswith("user "): + has_user_flag = True + break + i += 1 @@ if not has_user_flag and default_username: ssh_args.extend(["-l", default_username]) return ssh_args

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In packages/jumpstarter-driver-ssh/jumpstarter_driver_ssh/client.py around lines 88 to 109, the current detection for an already-specified username only checks for a standalone "-l" flag and misses inline forms like "-lroot" and -o options such as "-oUser=alice" or "-o User=alice"; update the detection to treat any ssh_options entry that starts with "-l" (and has characters after it) as specifying a user, and also inspect "-o" style options (both "-oUser=..." and "-o" followed by "User=...") to detect a User= assignment, then use that combined check to decide whether to skip adding the default username so the default is only injected when no user is present.

-Original file line number
+Diff line change
@@ Expand Up / @@ -80,6 +80,7 @@ General-purpose utility drivers: @@
     * **[Shell](shell.md)** (`jumpstarter-driver-shell`) - Shell command execution
     * **[TMT](tmt.md)** (`jumpstarter-driver-tmt`) - TMT (Test Management Tool) wrapper driver
+    * **[SSH](ssh.md)** (`jumpstarter-driver-ssh`) - SSH wrapper driver
     ```{toctree}
     :hidden:
@@ Expand All / @@ -101,6 +102,7 @@ gpiod.md @@
     ridesx.md
     sdwire.md
     shell.md
+    ssh.md
     snmp.md
     tasmota.md
     tmt.md
@@ Expand Down @@

Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		../../../../../packages/jumpstarter-driver-ssh/README.md
mangelajo marked this conversation as resolved. Show resolved Hide resolved

-Original file line number
+Diff line change
@@ -0,0 +1,92 @@
+    # SSHWrapper Driver
+    `jumpstarter-driver-ssh` provides SSH CLI functionality for Jumpstarter, allowing you to run SSH commands with configurable defaults and pass-through arguments.
+    ## Installation
+    ```shell
+    pip3 install --extra-index-url https://pkg.jumpstarter.dev/simple/ jumpstarter-driver-ssh
+    ```
+    ## Configuration
+    Example configuration:
+    ```yaml
+    export:
+      ssh:
+        type: jumpstarter_driver_ssh.driver.SSHWrapper
+        config:
+          default_username: "root"
+          ssh_command: "ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null"
+        children:
+          tcp:
+            type: jumpstarter_driver_network.driver.TcpNetwork
+            config:
+              host: "192.168.1.100"
+              port: 22
+    ```
+    ## Usage
+    The SSH driver provides a CLI command that accepts all standard SSH arguments:
+    ```bash
+    # Basic SSH connection (uses port forwarding by default)
+    j ssh
+    # SSH with direct TCP address
+    j ssh --direct
+    # SSH with specific user
+    j ssh -l myuser
+    # SSH with other flags
+    j ssh -i ~/.ssh/id_rsa
+    # Running a remote command
+    j ssh ls -la
+    ```
+    ## CLI Options
+    The SSH command supports the following options:
+    - `--direct`: Use direct TCP address (default is port forwarding)
+    All other arguments are passed directly to the SSH command. The driver uses the configured SSH command and default username from the driver configuration.
+    ### Username Handling
+    The driver supports multiple ways to specify the username:
+. **`-l username` flag**: Explicit username specification (takes precedence)
+. **Default username**: Used when no username is specified in arguments
+    If no `-l` flag or `user@hostname` format is provided, the default username from the driver configuration will be used automatically.
+    ## Dependencies
+    - `ssh`: Standard SSH client (usually pre-installed)
+    ## API Reference
+    ### Driver Methods
+    ```{eval-rst}
+    .. autoclass:: jumpstarter_driver_ssh.client.SSHWrapperClient()
+        :members: run
+    ```
+    ### Configuration Parameters
+    | Parameter        | Description                                                                                    | Type | Required | Default                                                                                    |
+    | ---------------- | ---------------------------------------------------------------------------------------------- | ---- | -------- | ------------------------------------------------------------------------------------------ |
+    | default_username | Default SSH username to use when no username is specified in the command                      | str  | no       | ""                                                                                         |
+    | ssh_command      | SSH command to use for connections                                                             | str  | no       | "ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o LogLevel=ERROR"      |
+    ### Required Children
+    - `tcp`: A TcpNetwork driver instance that provides the connection details (host and port)

-Original file line number
+Diff line change
@@ -0,0 +1,19 @@
+    apiVersion: jumpstarter.dev/v1alpha1
+    kind: ExporterConfig
+    metadata:
+      namespace: default
+      name: demo
+    endpoint: grpc.jumpstarter.192.168.0.203.nip.io:8082
+    token: "<token>"
+    export:
+      ssh:
+        type: jumpstarter_driver_ssh.driver.SSHWrapper
+        config:
+          default_username: "core"
+        children:
+          tcp:
+            type: jumpstarter_driver_network.driver.TcpNetwork
+            config:
+              host: "192.168.1.3"
+              port: 22

-Original file line number
+Diff line change
@@ -0,0 +1,183 @@
+    import shlex
+    import subprocess
+    from dataclasses import dataclass
+    from urllib.parse import urlparse
+    import click
+    from jumpstarter_driver_composite.client import CompositeClient
+    from jumpstarter_driver_network.adapters import TcpPortforwardAdapter
+    from jumpstarter.client.core import DriverMethodNotImplemented
+    @dataclass(kw_only=True)
+    class SSHWrapperClient(CompositeClient):
+        """
+        Client interface for SSHWrapper driver
+        This client provides methods to interact with SSH connections via CLI
+        """
+        def cli(self, click_group):
+            @click_group.command(context_settings={"ignore_unknown_options": True})
+            @click.option("--direct", is_flag=True, help="Use direct TCP address")
+            @click.argument("args", nargs=-1)
+            def ssh(direct, args):
+                """Run SSH command with arguments"""
+                result = self.run(direct, args)
+                self.logger.debug(f"SSH result: {result}")
+                if result != 0:
+                    click.get_current_context().exit(result)
+                return result
+            return ssh
+        # wrap the underlying tcp stream connections, so we can still use tcp forwarding or
+        # the fabric driver adapter on top of client.ssh
+        def stream(self, method="connect"):
+            return self.tcp.stream(method)
+        async def stream_async(self, method):
+            return await self.tcp.stream_async(method)
+        def run(self, direct, args):
+            """Run SSH command with the given parameters and arguments"""
+            # Get SSH command and default username from driver
+            ssh_command = self.call("get_ssh_command")
+            default_username = self.call("get_default_username")
+            if direct:
+                # Use direct TCP address
+                try:
+                    address = self.tcp.address()  # (format: "tcp://host:port")
+                    parsed = urlparse(address)
+                    host = parsed.hostname
+                    port = parsed.port
+                    if not host or not port:
+                        raise ValueError(f"Invalid address format: {address}")
+                    self.logger.debug(f"Using direct TCP connection for SSH - host: {host}, port: {port}")
+                    return self._run_ssh_local(host, port, ssh_command, default_username, args)
+                except (DriverMethodNotImplemented, ValueError) as e:
+                    self.logger.error(f"Direct address connection failed ({e}), falling back to SSH port forwarding")
+                    return self.run(False, args)
+            else:
+                # Use SSH port forwarding (default behavior)
+                self.logger.debug("Using SSH port forwarding for SSH connection")
+                with TcpPortforwardAdapter(
+                    client=self.tcp,
+                ) as addr:
+                    host = addr[0]
+                    port = addr[1]
+                    self.logger.debug(f"SSH port forward established - host: {host}, port: {port}")
+                    return self._run_ssh_local(host, port, ssh_command, default_username, args)
+        def _run_ssh_local(self, host, port, ssh_command, default_username, args):
+            """Run SSH command with the given host, port, and arguments"""
+            # Build SSH command arguments
+            ssh_args = self._build_ssh_command_args(ssh_command, port, default_username, args)
+            # Separate SSH options from command arguments
+            ssh_options, command_args = self._separate_ssh_options_and_command_args(args)
+            # Build final SSH command
+            ssh_args = self._build_final_ssh_command(ssh_args, ssh_options, host, command_args)
+            # Execute the command
+            return self._execute_ssh_command(ssh_args)
+        def _build_ssh_command_args(self, ssh_command, port, default_username, args):
+            """Build initial SSH command arguments"""
+            # Split the SSH command into individual arguments
+            ssh_args = shlex.split(ssh_command)
+            # Add port if specified
+            if port and port != 22:
+                ssh_args.extend(["-p", str(port)])
+            # Check if user already provided a username with -l flag in SSH options only
+            # We need to separate SSH options from command args first to avoid false positives
+            ssh_options, _ = self._separate_ssh_options_and_command_args(args)
+            has_user_flag = any(
+                ssh_options[i] == "-l" and i + 1 < len(ssh_options)
+                for i in range(len(ssh_options))
+            )
+            # Add default username if no -l flag provided and we have a default
+            if not has_user_flag and default_username:
+                ssh_args.extend(["-l", default_username])
+            return ssh_args
+        def _separate_ssh_options_and_command_args(self, args):
+            """Separate SSH options from command arguments"""
+            # SSH flags that do not expect a parameter (simple flags)
+            ssh_flags_no_param = {
+                '-4', '-6', '-A', '-a', '-C', '-f', '-G', '-g', '-K', '-k', '-M', '-N',
+                '-n', '-q', '-s', '-T', '-t', '-V', '-v', '-X', '-x', '-Y', '-y'
+            }
+            # SSH flags that do expect a parameter
+            ssh_flags_with_param = {
+                '-B', '-b', '-c', '-D', '-E', '-e', '-F', '-I', '-i', '-J', '-L', '-l',
+                '-m', '-O', '-o', '-P', '-p', '-Q', '-R', '-S', '-W', '-w'
+            }
+            ssh_options = []
+            command_args = []
+            i = 0
+            while i < len(args):
+                arg = args[i]
+                if arg.startswith('-'):
+                    # Check if it's a known SSH option
+                    if arg in ssh_flags_no_param:
+                        # This is a simple SSH flag without parameter
+                        ssh_options.append(arg)
+                    elif arg in ssh_flags_with_param:
+                        # This is an SSH flag that expects a parameter
+                        ssh_options.append(arg)
+                        # If this option takes a value, add the next argument too
+                        if i + 1 < len(args) and not args[i + 1].startswith('-'):
+                            ssh_options.append(args[i + 1])
+                            i += 1
+                    else:
+                        # This is a command argument - everything from here on is part of the command
+                        command_args = args[i:]
+                        break
+                else:
+                    # This is a command argument - everything from here on is part of the command
+                    command_args = args[i:]
+                    break
+                i += 1
+            # Debug output
+            self.logger.debug(f"SSH options: {ssh_options}")
+            self.logger.debug(f"Command args: {command_args}")
+            return ssh_options, command_args
+        def _build_final_ssh_command(self, ssh_args, ssh_options, host, command_args):
+            """Build the final SSH command with all components"""
+            # Add SSH options
+            ssh_args.extend(ssh_options)
+            # Add hostname before command arguments
+            if host:
+                ssh_args.append(host)
+            # Add command arguments
+            ssh_args.extend(command_args)
+            self.logger.debug(f"Running SSH command: {ssh_args}")
+            return ssh_args
+        def _execute_ssh_command(self, ssh_args):
+            """Execute the SSH command and return the result"""
+            try:
+                result = subprocess.run(ssh_args)
+                return result.returncode
+            except FileNotFoundError:
+                self.logger.error(
+                    f"SSH command '{ssh_args[0]}' not found. Please ensure SSH is installed and available in PATH."
+                )
+                return 127  # Standard exit code for "command not found"

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SSHWrapper driver #647

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot Sep 29, 2025

Uh oh!

coderabbitai bot Sep 29, 2025

Uh oh!

Uh oh!

Uh oh!

-Original file line number
+Diff line change
@@ -0,0 +1,33 @@
+    from dataclasses import dataclass
+    from jumpstarter.common.exceptions import ConfigurationError
+    from jumpstarter.driver import Driver, export
+    @dataclass(kw_only=True)
+    class SSHWrapper(Driver):
+        """SSH wrapper driver for Jumpstarter that provides SSH CLI functionality"""
+        default_username: str = ""
+        ssh_command: str = "ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o LogLevel=ERROR"
+        def __post_init__(self):
+            if hasattr(super(), "__post_init__"):
+                super().__post_init__()
+            if "tcp" not in self.children:
+                raise ConfigurationError("'tcp' child is required via ref, or directly as a TcpNetwork driver instance")
+        @classmethod
+        def client(cls) -> str:
+            return "jumpstarter_driver_ssh.client.SSHWrapperClient"
+        @export
+        def get_default_username(self):
+            """Get default SSH username"""
+            return self.default_username
+        @export
+        def get_ssh_command(self):
+            """Get the SSH command to use"""
+            return self.ssh_command

Add SSHWrapper driver #647

Uh oh!

Add SSHWrapper driver #647

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!