Skip to content
This repository was archived by the owner on Jan 23, 2026. It is now read-only.

Commit 2e07d6a

Browse files
bennyzgithub-actions[bot]
authored andcommitted
flashers: add tests
Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com> (cherry picked from commit 2e92d93)
1 parent 41cf6cf commit 2e07d6a

2 files changed

Lines changed: 92 additions & 24 deletions

File tree

packages/jumpstarter-driver-flashers/jumpstarter_driver_flashers/client.py

Lines changed: 53 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ def flash(
9090
"""Flash image to DUT"""
9191
should_download_to_httpd = True
9292
image_url = ""
93+
original_http_url = None
9394
operator_scheme = None
9495
# initrmafs cannot handle https yet, fallback to using the exporter's http server
9596
if path.startswith(("http://", "https://")) and not force_exporter_http:
@@ -102,11 +103,12 @@ def flash(
102103
if path.startswith(("http://", "https://")) and bearer_token:
103104
parsed = urlparse(path)
104105
self.logger.info(f"Using Bearer token authentication for {parsed.netloc}")
106+
original_http_url = path
105107
operator = Operator(
106108
"http", root="/", endpoint=f"{parsed.scheme}://{parsed.netloc}", token=bearer_token
107109
)
108110
operator_scheme = "http"
109-
path = Path(urlparse(path).path)
111+
path = Path(parsed.path)
110112
else:
111113
path, operator, operator_scheme = operator_for_path(path)
112114
image_url = self.http.get_url() + "/" + path.name
@@ -121,7 +123,16 @@ def flash(
121123
# Start the storage write operation in the background
122124
storage_thread = threading.Thread(
123125
target=self._transfer_bg_thread,
124-
args=(path, operator, operator_scheme, os_image_checksum, self.http.storage, error_queue, image_url),
126+
args=(
127+
path,
128+
operator,
129+
operator_scheme,
130+
os_image_checksum,
131+
self.http.storage,
132+
error_queue,
133+
original_http_url,
134+
headers,
135+
),
125136
name="storage_transfer",
126137
)
127138
storage_thread.start()
@@ -247,7 +258,11 @@ def _curl_tls_args(self, insecure_tls: bool, stored_cacert: str | None) -> str:
247258
def _prepare_headers(self, headers: dict[str, str] | None, bearer_token: str | None) -> str:
248259
all_headers = headers.copy() if headers else {}
249260
if bearer_token:
250-
all_headers["Authorization"] = f"Bearer {bearer_token}"
261+
if any(k.lower() == "authorization" for k in all_headers.keys()):
262+
self.logger.warning("Authorization header provided - ignoring bearer token")
263+
else:
264+
all_headers["Authorization"] = f"Bearer {bearer_token}"
265+
251266
return self._curl_header_args(all_headers)
252267

253268
def _curl_header_args(self, headers: dict[str, str] | None) -> str:
@@ -417,6 +432,7 @@ def _transfer_bg_thread(
417432
to_storage: OpendalClient,
418433
error_queue,
419434
original_url: str | None = None,
435+
headers: dict[str, str] | None = None,
420436
):
421437
"""Transfer image to exporter storage in the background
422438
Args:
@@ -426,6 +442,7 @@ def _transfer_bg_thread(
426442
error_queue: Queue to put exceptions in if any
427443
known_hash: Known hash of the image
428444
original_url: Original URL for HTTP fallback
445+
headers: HTTP headers for requests
429446
"""
430447
self.logger.info(f"Writing image to storage in the background: {src_path}")
431448
try:
@@ -451,7 +468,9 @@ def _transfer_bg_thread(
451468
self.logger.info(f"Uploading image to storage: {filename}")
452469
to_storage.write_from_path(filename, src_path, src_operator)
453470

454-
metadata, metadata_json = self._create_metadata_and_json(src_operator, src_path, file_hash, original_url)
471+
metadata, metadata_json = self._create_metadata_and_json(
472+
src_operator, src_path, file_hash, original_url, headers
473+
)
455474
metadata_file = filename + ".metadata"
456475
to_storage.write_bytes(metadata_file, metadata_json.encode(errors="ignore"))
457476

@@ -682,7 +701,9 @@ def _parse_headers(self, headers: list[str]) -> dict[str, str]:
682701
Raises:
683702
click.ClickException: If header format is invalid
684703
"""
685-
header_map = {}
704+
token_re = re.compile(r"^[!#$%&'*+\-.^_`|~0-9A-Za-z]+$")
705+
header_map: dict[str, str] = {}
706+
seen: set[str] = set()
686707
for h in headers:
687708
if ":" not in h:
688709
raise click.ClickException(f"Invalid header format: {h!r}. Expected 'Key: Value'.")
@@ -694,9 +715,36 @@ def _parse_headers(self, headers: list[str]) -> dict[str, str]:
694715
if not key:
695716
raise click.ClickException(f"Invalid header key in: {h!r}")
696717

718+
if not token_re.match(key):
719+
raise click.ClickException(f"Invalid header name '{key}': must be an HTTP token (RFC7230)")
720+
if any(c in ("\r", "\n") for c in key) or any(c in ("\r", "\n") for c in value):
721+
raise click.ClickException("Header names/values must not contain CR/LF")
722+
kl = key.lower()
723+
if kl in seen:
724+
raise click.ClickException(f"Duplicate header '{key}'")
725+
seen.add(kl)
697726
header_map[key] = value
727+
698728
return header_map
699729

730+
def _validate_bearer_token(self, token: str | None) -> str | None:
731+
if token is None:
732+
return None
733+
734+
token = token.strip()
735+
if not token:
736+
raise click.ClickException("Bearer token cannot be empty")
737+
738+
# RFC 6750 allows token68 format (base64url-encoded) or other token formats
739+
# Basic validation: printable ASCII excluding whitespace and special chars that could cause issues
740+
if not all(32 < ord(c) < 127 and c not in ' "\\' for c in token):
741+
raise click.ClickException("Bearer token contains invalid characters")
742+
743+
if len(token) > 4096:
744+
raise click.ClickException("Bearer token is too long (max 4096 characters)")
745+
746+
return token
747+
700748
def cli(self):
701749
@driver_click_group(self)
702750
def base():
@@ -806,22 +854,3 @@ def _get_decompression_command(filename_or_url) -> str:
806854
elif filename.endswith(".xz"):
807855
return "xzcat |"
808856
return ""
809-
810-
811-
def _validate_bearer_token(self, token: str | None) -> str | None:
812-
if token is None:
813-
return None
814-
815-
token = token.strip()
816-
if not token:
817-
raise click.ClickException("Bearer token cannot be empty")
818-
819-
# RFC 6750 allows token68 format (base64url-encoded) or other token formats
820-
# Basic validation: printable ASCII excluding whitespace and special chars that could cause issues
821-
if not all(32 < ord(c) < 127 and c not in ' "\\' for c in token):
822-
raise click.ClickException("Bearer token contains invalid characters")
823-
824-
if len(token) > 4096:
825-
raise click.ClickException("Bearer token is too long (max 4096 characters)")
826-
827-
return token
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import click
2+
import pytest
3+
4+
from .client import BaseFlasherClient
5+
6+
7+
class MockFlasherClient(BaseFlasherClient):
8+
"""Mock client for testing without full initialization"""
9+
10+
def __init__(self):
11+
self._manifest = None
12+
self._console_debug = False
13+
self.logger = type(
14+
"MockLogger", (), {"warning": lambda msg: None, "info": lambda msg: None, "error": lambda msg: None}
15+
)()
16+
17+
18+
def test_validate_bearer_token_fails_invalid():
19+
"""Test bearer token validation fails with invalid tokens"""
20+
client = MockFlasherClient()
21+
22+
with pytest.raises(click.ClickException, match="Bearer token cannot be empty"):
23+
client._validate_bearer_token("")
24+
25+
with pytest.raises(click.ClickException, match="Bearer token contains invalid characters"):
26+
client._validate_bearer_token("token with spaces")
27+
28+
with pytest.raises(click.ClickException, match="Bearer token contains invalid characters"):
29+
client._validate_bearer_token('token"with"quotes')
30+
31+
32+
def test_curl_header_args_handles_quotes():
33+
"""Test curl header formatting safely handles quotes"""
34+
client = MockFlasherClient()
35+
36+
result = client._curl_header_args({"Authorization": "Bearer abc'def"})
37+
assert "'\"'\"'" in result
38+
assert result.startswith("-H '")
39+
assert result.endswith("'")

0 commit comments

Comments
 (0)