Skip to content

ridesx: support http and insecure https #783

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bennyz wants to merge 3 commits into
base: main
Choose a base branch
from
Open

ridesx: support http and insecure https #783

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
34e3864
ridesx: support http and insecure https
bennyz Dec 17, 2025
df665fc
ridesx: download to temp file
bennyz Dec 23, 2025
8b0744b
Merge branch 'main' into ridesx-insecure
bennyz Jan 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 86 additions & 5 deletions packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import ssl
import tempfile
from dataclasses import dataclass
from pathlib import Path
from typing import Dict, Optional
from urllib.parse import urlparse

import aiohttp
import click
from jumpstarter_driver_composite.client import CompositeClient
from jumpstarter_driver_opendal.client import FlasherClient, operator_for_path
from jumpstarter_driver_power.client import PowerClient
Expand All @@ -22,10 +27,51 @@ def __post_init__(self):
def boot_to_fastboot(self):
return self.call("boot_to_fastboot")

def _upload_file_if_needed(self, file_path: str, operator: Operator | None = None) -> str:
def _is_http_url(self, path: str) -> bool:
"""Check if the path is an HTTP or HTTPS URL."""
return isinstance(path, str) and path.startswith(("http://", "https://"))

def _download_http_to_storage(self, url: str, storage, filename: str, insecure_tls: bool = False) -> None:
async def _download():
parsed = urlparse(url)
if parsed.scheme == "http" or insecure_tls:
ssl_context: ssl.SSLContext | bool = False
else:
ssl_context = True

connector = aiohttp.TCPConnector(ssl=ssl_context)
async with aiohttp.ClientSession(connector=connector) as session:
async with session.get(url) as response:
response.raise_for_status()
with tempfile.NamedTemporaryFile(delete=False, dir="/var/tmp") as f:
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# Find and examine the client.py file in the jumpstarter-driver-ridesx package
find . -path "*/packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client.py" 2>/dev/null | head -1 | xargs ls -la 2>/dev/null || echo "File path not found, searching by name..."

# Try alternative search
find . -name "client.py" -path "*ridesx*" 2>/dev/null | head -5

Repository: jumpstarter-dev/jumpstarter

Length of output: 265

🏁 Script executed:

#!/bin/bash

# List files in the repository to find the right structure
git ls-files | grep -E "ridesx|client\.py" | head -20

Repository: jumpstarter-dev/jumpstarter

Length of output: 1408

🏁 Script executed:

#!/bin/bash

# Look at the file around line 46
find . -path "*jumpstarter-driver-ridesx*/client.py" -exec cat -n {} + 2>/dev/null | sed -n '40,55p'

Repository: jumpstarter-dev/jumpstarter

Length of output: 884

🏁 Script executed:

#!/bin/bash

# Search for all uses of NamedTemporaryFile in the codebase
rg -n "NamedTemporaryFile" --type=py -B2 -A2

Repository: jumpstarter-dev/jumpstarter

Length of output: 21514

🏁 Script executed:

#!/bin/bash

# First, let's find the actual file
find . -type f -name "client.py" | xargs grep -l "NamedTemporaryFile" 2>/dev/null

Repository: jumpstarter-dev/jumpstarter

Length of output: 281

🏁 Script executed:

#!/bin/bash

# Search for the ridesx client.py file
rg "NamedTemporaryFile" --type py -l

Repository: jumpstarter-dev/jumpstarter

Length of output: 970

🏁 Script executed:

#!/bin/bash
# Find all NamedTemporaryFile usages and their context
rg -n "NamedTemporaryFile" --type=py -B3 -A3 | head -60

Repository: jumpstarter-dev/jumpstarter

Length of output: 5456

🏁 Script executed:

#!/bin/bash
# Specifically look for the file mentioned in the review
if [ -f "packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client.py" ]; then
  echo "=== File exists, showing context around line 46 ==="
  sed -n '40,55p' "packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client.py" | cat -n
else
  echo "File not found at expected path"
  find . -name "client.py" -path "*ridesx*" 2>/dev/null
fi

Repository: jumpstarter-dev/jumpstarter

Length of output: 936

🏁 Script executed:

#!/bin/bash
# Check how tempfile is used throughout the codebase
echo "=== All tempfile uses in the codebase ==="
rg "tempfile\." --type=py | head -30

Repository: jumpstarter-dev/jumpstarter

Length of output: 3959

Remove hardcoded /var/tmp directory.

The hardcoded dir="/var/tmp" parameter restricts portability unnecessarily. Python's tempfile module automatically searches through /tmp, /var/tmp, and /usr/tmp when no directory is specified, adapting to each deployment environment. Remove the dir parameter to align with the codebase pattern (all other NamedTemporaryFile calls in the repository omit this) and allow the code to work across containers, Windows, macOS, and restricted environments.

🤖 Prompt for AI Agents 
In packages/jumpstarter-driver-ridesx/jumpstarter_driver_ridesx/client.py around
line 46, the call to tempfile.NamedTemporaryFile uses a hardcoded
dir="/var/tmp", which reduces portability; remove the dir parameter so
NamedTemporaryFile uses the system default temp directory (i.e., call
tempfile.NamedTemporaryFile(...) without dir) to match the repository pattern
and allow the code to run correctly across containers, Windows, macOS, and
restricted environments.

async for chunk in response.content.iter_chunked(65536):
f.write(chunk)
return Path(f.name)

tmp_path = self.portal.call(_download)
try:
storage.write_from_path(filename, tmp_path)
finally:
tmp_path.unlink()

def _upload_file_if_needed(
self, file_path: str, operator: Operator | None = None, insecure_tls: bool = False
) -> str:
if not file_path or not file_path.strip():
raise ValueError("File path cannot be empty. Please provide a valid file path.")

if self._is_http_url(file_path) and operator is None:
parsed = urlparse(file_path)
is_insecure_http = parsed.scheme == "http"

# use aiohttp for: http:// URLs, or https:// with insecure_tls
if is_insecure_http or insecure_tls:
filename = Path(parsed.path).name
self.logger.info(f"Downloading {file_path} to storage as {filename}")
self._download_http_to_storage(file_path, self.storage, filename, insecure_tls=insecure_tls)
return filename

# use opendal for local files, https:// (secure), and other schemes
if operator is None:
path_buf, operator, operator_scheme = operator_for_path(file_path)
else:
Expand All @@ -46,12 +92,18 @@ def _upload_file_if_needed(self, file_path: str, operator: Operator | None = Non

return filename

def flash_images(self, partitions: Dict[str, str], operators: Optional[Dict[str, Operator]] = None):
def flash_images(
self,
partitions: Dict[str, str],
operators: Optional[Dict[str, Operator]] = None,
insecure_tls: bool = False,
):
"""Flash images to specified partitions

Args:
partitions: Dictionary mapping partition names to file paths
operators: Optional dictionary mapping partition names to operators
insecure_tls: Skip TLS certificate verification for HTTPS URLs
"""
if not partitions:
raise ValueError("At least one partition must be provided")
Expand All @@ -62,7 +114,7 @@ def flash_images(self, partitions: Dict[str, str], operators: Optional[Dict[str,
for partition, file_path in partitions.items():
self.logger.info(f"Processing {partition} image: {file_path}")
operator = operators.get(partition)
remote_files[partition] = self._upload_file_if_needed(file_path, operator)
remote_files[partition] = self._upload_file_if_needed(file_path, operator, insecure_tls=insecure_tls)

self.logger.info("Checking for fastboot devices on Exporter...")
detection_result = self.call("detect_fastboot_device", 5, 2.0)
Expand All @@ -84,6 +136,7 @@ def flash(
target: str | None = None,
operator: Operator | Dict[str, Operator] | None = None,
compression=None,
insecure_tls: bool = False,
):
if isinstance(path, dict):
partitions = path
Expand All @@ -109,7 +162,7 @@ def flash(

self.boot_to_fastboot()

result = self.flash_images(partitions, operators)
result = self.flash_images(partitions, operators, insecure_tls=insecure_tls)

self.logger.info("flash operation completed successfully")

Expand All @@ -130,7 +183,35 @@ def base():
pass

for name, cmd in generic_cli.commands.items():
base.add_command(cmd, name=name)
if name != "flash":
base.add_command(cmd, name=name)

@base.command()
@click.argument("file", nargs=-1, required=False)
@click.option(
"--target",
"-t",
"target_specs",
multiple=True,
help="name:file",
)
@click.option("--insecure-tls", is_flag=True, help="Skip TLS certificate verification")
def flash(file, target_specs, insecure_tls):
"""Flash image to DUT"""
if target_specs:
mapping: dict[str, str] = {}
for spec in target_specs:
if ":" not in spec:
raise click.ClickException(f"Invalid target spec '{spec}', expected name:file")
name, img = spec.split(":", 1)
mapping[name] = img
self.flash(mapping, insecure_tls=insecure_tls)
return

if not file:
raise click.ClickException("FILE argument is required unless --target/-t is used")

self.flash(file[0], target=None, insecure_tls=insecure_tls)

@base.command()
def boot_to_fastboot():
Expand Down