From 99f1ab6382020e64304a5b43d0669ec9d4783ba0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20A=2E=20Rodr=C3=ADguez?= Date: Thu, 3 Sep 2026 00:25:51 +0200 Subject: [PATCH 1/3] Download Rosim JAR from a pinned release asset instead of the GitHub API Every cold container raised KeyError: 'assets' because the response body from the unauthenticated api.github.com releases endpoint had no "assets" key. That endpoint is rate-limited to 60 requests/hour per IP, which on shared CI runner IPs is the near-certain cause; the code discarded the status code, so this was never confirmed from a log. A cached JAR short-circuits the download, which is why this passed locally and failed everywhere else. Fetch from a pinned release download URL, which is not rate-limited, and verify the download against a known SHA-256. The file is staged through a temporary name that deliberately does not match the rosim*.jar cache glob, so a process killed mid-download cannot leave a truncated file to be picked up as a valid cached JAR on the next run. The pinned tag is decoupled from this package's version, which means upload_rosim_to_release.yml's per-release upload is not what this code reads. Bumping the JAR without bumping the constants would otherwise serve the previous version with a still-matching checksum, so tests assert the pinned name and digest against the JAR committed in bin. --- .../entities/assets/pyjnius_loader.py | 137 +++++--- unit_test/entities/test_pyjnius_loader.py | 295 ++++++++++++++++++ 2 files changed, 393 insertions(+), 39 deletions(-) create mode 100644 unit_test/entities/test_pyjnius_loader.py diff --git a/src/omotes_simulator_core/entities/assets/pyjnius_loader.py b/src/omotes_simulator_core/entities/assets/pyjnius_loader.py index c75de8b1..a8c885b4 100644 --- a/src/omotes_simulator_core/entities/assets/pyjnius_loader.py +++ b/src/omotes_simulator_core/entities/assets/pyjnius_loader.py @@ -14,13 +14,54 @@ # along with this program. If not, see . """Binding to Rosim through Pyjnius.""" +import glob +import hashlib import logging import os +import tempfile +import urllib.request from typing import Callable JavaClass = Callable logger = logging.getLogger(__name__) +# These three values describe one specific published Rosim JAR asset and must be bumped +# together whenever a new Rosim JAR is released. +# +# The tag names the release whose asset is read, not the release this code ships in. +# `upload_rosim_to_release.yml` attaches a JAR to every release, but this code does not read +# those attachments: bumping the workflow's output alone leaves the pin serving the old JAR +# with its checksum still matching. +# +# When bumping, establish the digest from a trusted build of the JAR rather than from a copy +# downloaded through this pin. A digest taken from the downloaded file only restates that +# file's contents and would attest to a tampered artifact just as readily as a genuine one. +ROSIM_RELEASE_TAG = "0.0.30" +ROSIM_JAR_NAME = "rosim-batch-1.2.0.jar" +ROSIM_JAR_SHA256 = "8483fb9eb68608cffc3b72279b19947b2c1cfd9f936377c278ee0f3291b68547" + +# Derived at import; patching a constant above in a test will not change it. +ROSIM_JAR_URL = ( + f"https://github.com/Project-OMOTES/simulator-core/releases/download/" + f"{ROSIM_RELEASE_TAG}/{ROSIM_JAR_NAME}" +) + +_HASH_CHUNK_SIZE = 1024 * 1024 + + +def _get_bin_path() -> str: + """Return the directory that holds the bundled JAR files.""" + return os.path.join(os.path.dirname(__file__), "bin") + + +def _sha256_of_file(file_path: str) -> str: + """Return the hex SHA-256 digest of a file, read in chunks to bound memory use.""" + digest = hashlib.sha256() + with open(file_path, "rb") as file_handle: + for chunk in iter(lambda: file_handle.read(_HASH_CHUNK_SIZE), b""): + digest.update(chunk) + return digest.hexdigest() + class PyjniusLoader: """Class to load Pyjnius and connect to Rosim. @@ -42,60 +83,78 @@ def __init__(self) -> None: This function should only be called ONCE. Do not use construct this class directly but rather use `PyjniusLoader.get_loader`. """ - path = os.path.dirname(__file__) + bin_path = _get_bin_path() import jnius_config # noqa + # Resolved through _get_bin_path so the classpath cannot drift from the folder + # download_rosim_jar writes into. self.rosim_jar = self.download_rosim_jar() - jnius_config.add_classpath(os.path.join(path, "bin/jfxrt.jar")) - jnius_config.add_classpath(os.path.join(path, "bin", self.rosim_jar)) + jnius_config.add_classpath(os.path.join(bin_path, "jfxrt.jar")) + jnius_config.add_classpath(os.path.join(bin_path, self.rosim_jar)) self.loaded_classes = {} @staticmethod def download_rosim_jar() -> str: - """Download the Rosim JAR files. - - This function will download the required Rosim JAR files into the `bin` folder. - It will download it from the latest release on github of the simulator-core repository. - If there is already a rosim jar in the bin folder it will not download it and use that one. - If you want the downloaded the latest one, simply delete your local copy. - We have implemented this to bypass the 100 mb size limit on pypi. - It returns the name of the downloaded JAR file. - """ - import glob - import urllib.request + """Ensure the Rosim JAR is present in the `bin` folder and return its file name. - import requests # type: ignore[import-untyped] + The JAR is downloaded at runtime rather than shipped inside the wheel because it + exceeds the 100 MB per-file size limit on PyPI. - # First a check if there are already jar files present. - base_path = os.path.dirname(__file__) - bin_path = os.path.join(base_path, "bin") + If a `rosim*.jar` is already present in the `bin` folder, no download happens and + that file is used. To pick up a newer Rosim JAR, bump the module-level constants; + deleting the local copy only re-fetches the same pinned asset. + + The JAR is fetched from a pinned release asset URL, written to a temporary file in + the `bin` folder and verified against `ROSIM_JAR_SHA256`. Only a file with a matching + digest is moved into its final place, so an interrupted or corrupted download can + never be picked up as a valid cached JAR. A RuntimeError is raised when the digest + does not match. + """ + bin_path = _get_bin_path() + # First a check if there are already jar files present. jar_files = glob.glob(os.path.join(bin_path, "rosim*.jar")) if jar_files: logger.debug("Rosim JAR files already present, skipping download.") logger.debug(f"Using Rosim JAR file: {jar_files[0]}") - return jar_files[0] - - repo = "Project-OMOTES/simulator-core" - api_url = f"https://api.github.com/repos/{repo}/releases/latest" - - response = requests.get(api_url) - release_data = response.json() - assets = release_data["assets"] - for asset in assets: - if "rosim" in asset["name"]: - # downloading the jar file from github releeases - logger.debug("Downloading Rosim JAR files from GitHub") - url = asset["browser_download_url"] - jar_file = os.path.join(bin_path, asset["name"]) - response_url = urllib.request.urlretrieve( - url, - jar_file, + return os.path.basename(jar_files[0]) + + logger.debug("Downloading Rosim JAR files from GitHub") + os.makedirs(bin_path, exist_ok=True) + final_path = os.path.join(bin_path, ROSIM_JAR_NAME) + + # Download next to the target so the final move is an atomic rename on the same volume. + # The temporary name must never match the `rosim*.jar` cache glob above: a partial + # download left behind by a killed process would otherwise be cached as a valid JAR. + temp_fd, temp_path = tempfile.mkstemp(dir=bin_path, prefix=".rosim-download-") + try: + os.close(temp_fd) + # mkstemp creates the file 0600 and os.replace preserves that, where a plain + # download would have taken the umask default. Re-apply the umask so the JAR is + # readable by whoever the JVM runs as, without widening past what the umask allows. + umask = os.umask(0) + os.umask(umask) + os.chmod(temp_path, 0o666 & ~umask) + urllib.request.urlretrieve(ROSIM_JAR_URL, temp_path) + actual_hash = _sha256_of_file(temp_path) + if actual_hash != ROSIM_JAR_SHA256: + raise RuntimeError( + f"Checksum mismatch for downloaded Rosim JAR from {ROSIM_JAR_URL}: " + f"expected SHA-256 {ROSIM_JAR_SHA256}, got {actual_hash}." ) - if response_url is None: - raise RuntimeError("Failed to download Rosim JAR files.") - return str(asset["name"]) - raise RuntimeError("Failed to find Rosim JAR files in GitHub releases.") + os.replace(temp_path, final_path) + except BaseException: + try: + if os.path.exists(temp_path): + os.remove(temp_path) + except OSError: + # A failure to clean up must not replace the error that caused it, which + # carries the reason the download is unusable. + logger.warning(f"Failed to remove temporary download {temp_path}.") + raise + + logger.debug(f"Using Rosim JAR file: {final_path}") + return ROSIM_JAR_NAME def load_class(self, classpath: str) -> JavaClass: """Load a Java class. diff --git a/unit_test/entities/test_pyjnius_loader.py b/unit_test/entities/test_pyjnius_loader.py new file mode 100644 index 00000000..0f1668ba --- /dev/null +++ b/unit_test/entities/test_pyjnius_loader.py @@ -0,0 +1,295 @@ +# Copyright (c) 2023. Deltares & TNO +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +"""Test the Rosim JAR download logic of the Pyjnius loader.""" +import fnmatch +import glob +import hashlib +import os +import tempfile +import unittest +from typing import Optional +from unittest.mock import patch + +from omotes_simulator_core.entities.assets import pyjnius_loader +from omotes_simulator_core.entities.assets.pyjnius_loader import PyjniusLoader + +PAYLOAD = b"this is not really a jar file" * 100 +PAYLOAD_SHA256 = hashlib.sha256(PAYLOAD).hexdigest() + +# A git-LFS pointer stub is a few hundred bytes of text; a real JAR is over 100 MB. +_LFS_POINTER_MAX_SIZE = 1024 + + +def _lfs_pointer_digest(file_path: str) -> Optional[str]: + """Return the sha256 oid recorded in a git-LFS pointer stub, or None for real content.""" + if os.path.getsize(file_path) > _LFS_POINTER_MAX_SIZE: + return None + try: + with open(file_path, "r", encoding="utf-8") as file_handle: + lines = file_handle.read().splitlines() + except UnicodeDecodeError: + return None + + if not lines or not lines[0].startswith("version https://git-lfs.github.com/spec/"): + return None + for line in lines: + if line.startswith("oid sha256:"): + return line.split("oid sha256:", 1)[1].strip() + return None + + +class DownloadRosimJarTest(unittest.TestCase): + """Testcase for PyjniusLoader.download_rosim_jar.""" + + def setUp(self) -> None: + """Point the loader at an empty temporary bin folder.""" + self._temp_dir = tempfile.TemporaryDirectory() + self.bin_dir = self._temp_dir.name + patcher = patch.object(pyjnius_loader, "_get_bin_path", return_value=self.bin_dir) + patcher.start() + self.addCleanup(patcher.stop) + self.addCleanup(self._temp_dir.cleanup) + + def _write_payload(self, _url, target_path): + """Stand in for urllib.request.urlretrieve by writing the known payload.""" + with open(target_path, "wb") as file_handle: + file_handle.write(PAYLOAD) + return target_path, None + + def test_cache_hit_skips_download(self): + """An existing rosim JAR short-circuits the download and yields its bare name.""" + cached_name = "rosim-test-9.9.9.jar" + with open(os.path.join(self.bin_dir, cached_name), "wb") as file_handle: + file_handle.write(b"cached") + + with patch.object(pyjnius_loader.urllib.request, "urlretrieve") as fake_retrieve: + result = PyjniusLoader.download_rosim_jar() + + self.assertEqual(result, cached_name) + self.assertEqual(fake_retrieve.call_count, 0) + self.assertEqual(sorted(os.listdir(self.bin_dir)), [cached_name]) + + def test_successful_download_moves_file_into_place(self): + """A download whose digest matches lands at the final path and leaves no temp file.""" + with patch.object(pyjnius_loader, "ROSIM_JAR_SHA256", PAYLOAD_SHA256): + with patch.object( + pyjnius_loader.urllib.request, "urlretrieve", side_effect=self._write_payload + ) as fake_retrieve: + result = PyjniusLoader.download_rosim_jar() + + self.assertEqual(result, pyjnius_loader.ROSIM_JAR_NAME) + self.assertEqual(fake_retrieve.call_count, 1) + self.assertEqual(fake_retrieve.call_args[0][0], pyjnius_loader.ROSIM_JAR_URL) + self.assertEqual(sorted(os.listdir(self.bin_dir)), [pyjnius_loader.ROSIM_JAR_NAME]) + with open(os.path.join(self.bin_dir, pyjnius_loader.ROSIM_JAR_NAME), "rb") as file_handle: + self.assertEqual(file_handle.read(), PAYLOAD) + + def test_hash_mismatch_raises_and_leaves_nothing_behind(self): + """A digest mismatch raises RuntimeError and no file is kept, temporary or final.""" + wrong_hash = "0" * 64 + with patch.object(pyjnius_loader, "ROSIM_JAR_SHA256", wrong_hash): + with patch.object( + pyjnius_loader.urllib.request, "urlretrieve", side_effect=self._write_payload + ): + with self.assertRaises(RuntimeError) as context: + PyjniusLoader.download_rosim_jar() + + message = str(context.exception) + self.assertIn(wrong_hash, message) + self.assertIn(PAYLOAD_SHA256, message) + self.assertFalse(os.path.exists(os.path.join(self.bin_dir, pyjnius_loader.ROSIM_JAR_NAME))) + self.assertEqual(os.listdir(self.bin_dir), []) + + def test_download_error_propagates_and_leaves_nothing_behind(self): + """A failing download propagates and removes the partial temporary file.""" + + def failing_retrieve(_url, target_path): + with open(target_path, "wb") as file_handle: + file_handle.write(b"partial") + raise OSError("connection reset") + + with patch.object( + pyjnius_loader.urllib.request, "urlretrieve", side_effect=failing_retrieve + ): + with self.assertRaises(OSError): + PyjniusLoader.download_rosim_jar() + + self.assertEqual(os.listdir(self.bin_dir), []) + + def test_partial_download_is_never_visible_to_the_cache_glob(self): + """While a download is in flight its file must not match the `rosim*.jar` cache glob. + + A process killed mid-download leaves the partial file behind. If that name matched the + glob, the next run would short-circuit on it and treat a truncated file as a valid + cached JAR forever, so the temporary name is load-bearing rather than cosmetic. + """ + targets: list[str] = [] + + def retrieve_and_inspect(_url, target_path): + targets.append(target_path) + with open(target_path, "wb") as file_handle: + file_handle.write(b"partial") + raise OSError("killed mid-download") + + with patch.object( + pyjnius_loader.urllib.request, "urlretrieve", side_effect=retrieve_and_inspect + ): + with self.assertRaises(OSError): + PyjniusLoader.download_rosim_jar() + + self.assertEqual(len(targets), 1) + # Assert on the name itself: globbing the bin folder would also come up empty if the + # download were written somewhere else entirely, passing for the wrong reason. + self.assertEqual(os.path.dirname(targets[0]), self.bin_dir) + self.assertFalse( + fnmatch.fnmatch(os.path.basename(targets[0]), "rosim*.jar"), + f"in-flight download {os.path.basename(targets[0])} matches the cache glob", + ) + + +class LfsPointerDigestTest(unittest.TestCase): + """Testcase for the git-LFS pointer parsing used by the pinned-constant guard. + + The pinned-digest test below reads a real JAR, so these branches are never exercised + there. Without direct coverage a broken parser would silently make that guard hash the + wrong bytes rather than fail. + """ + + def setUp(self) -> None: + """Provide a temporary folder to write pointer fixtures into.""" + self._temp_dir = tempfile.TemporaryDirectory() + self.addCleanup(self._temp_dir.cleanup) + + def _write(self, text: str, name: str = "stub") -> str: + """Write a fixture file and return its path.""" + path = os.path.join(self._temp_dir.name, name) + with open(path, "w", encoding="utf-8") as file_handle: + file_handle.write(text) + return path + + def test_reads_oid_from_pointer(self): + """A well-formed pointer yields the digest recorded in its oid line.""" + oid = "a" * 64 + path = self._write( + "version https://git-lfs.github.com/spec/v1\n" f"oid sha256:{oid}\nsize 129225676\n" + ) + self.assertEqual(_lfs_pointer_digest(path), oid) + + def test_returns_none_for_large_file(self): + """Content above the pointer size limit is rejected on size alone. + + The fixture is a valid pointer padded past the limit, so only the size guard can + reject it; a fixture that also failed the header check would pass even with the + size guard removed. + """ + oid = "c" * 64 + padding = "\n" * _LFS_POINTER_MAX_SIZE + path = self._write( + "version https://git-lfs.github.com/spec/v1\n" f"oid sha256:{oid}\n{padding}" + ) + self.assertGreater(os.path.getsize(path), _LFS_POINTER_MAX_SIZE) + self.assertIsNone(_lfs_pointer_digest(path)) + + def test_returns_none_for_binary_content(self): + """A small binary file is not a pointer and must not raise while being checked.""" + path = os.path.join(self._temp_dir.name, "binary") + with open(path, "wb") as file_handle: + file_handle.write(b"\xff\xfe\x00\x01") + self.assertIsNone(_lfs_pointer_digest(path)) + + def test_returns_none_without_pointer_header(self): + """Small text lacking the LFS version header is not a pointer.""" + path = self._write(f"oid sha256:{'b' * 64}\n") + self.assertIsNone(_lfs_pointer_digest(path)) + + def test_returns_none_when_oid_line_missing(self): + """A pointer header with no oid line yields no digest rather than a wrong one.""" + path = self._write("version https://git-lfs.github.com/spec/v1\nsize 42\n") + self.assertIsNone(_lfs_pointer_digest(path)) + + +class PinnedConstantsMatchRepositoryTest(unittest.TestCase): + """Guard the pinned constants against drifting from the JAR committed in `bin`. + + `upload_rosim_to_release.yml` publishes whatever `rosim*.jar` sits in `bin`, while the + loader reads a hardcoded release asset. Nothing links the two, so swapping the JAR without + updating the constants leaves the pin serving the previous JAR whose checksum still + matches -- a silently wrong Rosim version rather than a visible failure. Replacing the JAR + without updating `ROSIM_JAR_NAME` and `ROSIM_JAR_SHA256` fails here instead. + + Two limits are deliberate. `ROSIM_RELEASE_TAG` is checked against nothing, so bumping the + tag alone is not caught; it points the URL at a possibly absent asset and surfaces as a + download failure at runtime, which is loud enough. And an empty `bin` skips rather than + fails, because `pyproject.toml` does not ship `rosim-batch-*.jar` as package data: an + installed package has no JAR until the first download, and only a repository checkout + carries the committed one these tests compare against. + """ + + def setUp(self) -> None: + """Locate the JAR committed in the real `bin` folder.""" + self.bin_dir = pyjnius_loader._get_bin_path() + self.jars = sorted( + os.path.basename(path) for path in glob.glob(os.path.join(self.bin_dir, "rosim*.jar")) + ) + + def test_at_most_one_rosim_jar_is_committed(self): + """Several candidate JARs make the loader's cache short-circuit ambiguous. + + Zero JARs is not a drift signal: `pyproject.toml` does not ship `rosim-batch-*.jar` + as package data, so an installed package legitimately has none until the first + download. Only the ambiguous case is a failure. + """ + self.assertLessEqual( + len(self.jars), 1, f"expected at most one rosim JAR in bin, found {self.jars}" + ) + + def _single_jar(self) -> str: + """Return the committed rosim JAR, skipping when there is no single one to compare. + + An empty `bin` is legitimate outside a repository checkout; an ambiguous one is + reported by the count test above rather than again here. + """ + if len(self.jars) != 1: + self.skipTest(f"no single rosim JAR in bin to compare against, found {self.jars}") + return self.jars[0] + + def test_pinned_name_matches_committed_jar(self): + """ROSIM_JAR_NAME must name the JAR actually shipped in `bin`.""" + self.assertEqual( + pyjnius_loader.ROSIM_JAR_NAME, + self._single_jar(), + "ROSIM_JAR_NAME does not match the JAR in bin; bump the pinned constants " + "(tag, name and SHA-256) together with the JAR.", + ) + + def test_pinned_digest_matches_committed_jar(self): + """ROSIM_JAR_SHA256 must match the committed JAR's content digest. + + On a checkout without git-LFS content the file is a small pointer stub, whose + `oid sha256:` line records the real digest; hashing the stub would compare the wrong + bytes. The stub is read in that case so the check stays meaningful either way. + """ + jar_path = os.path.join(self.bin_dir, self._single_jar()) + digest = _lfs_pointer_digest(jar_path) + if digest is None: + digest = pyjnius_loader._sha256_of_file(jar_path) + + self.assertEqual( + digest, + pyjnius_loader.ROSIM_JAR_SHA256, + "ROSIM_JAR_SHA256 does not match the JAR in bin; bump the pinned constants " + "(tag, name and SHA-256) together with the JAR.", + ) From 81ee8ba224f3176e359ea5b0e20c3191f6ccca3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20A=2E=20Rodr=C3=ADguez?= Date: Thu, 3 Sep 2026 12:48:54 +0200 Subject: [PATCH 2/3] Address review comments on the Rosim JAR download Correct the docstring's upgrade instructions: the cache glob matches any rosim*.jar, so bumping the constants alone keeps using an older local copy -- both a bump and a delete are needed. Replace the umask read with copying the mode of a sibling shipped JAR. Reading the umask means setting it to 0 and restoring it, which mutates process-global state that a library should not touch. Parameterize the debug logs so their arguments are not formatted when the level is disabled, and keep the traceback when temporary-file cleanup fails. --- .../entities/assets/pyjnius_loader.py | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/omotes_simulator_core/entities/assets/pyjnius_loader.py b/src/omotes_simulator_core/entities/assets/pyjnius_loader.py index a8c885b4..e51c7604 100644 --- a/src/omotes_simulator_core/entities/assets/pyjnius_loader.py +++ b/src/omotes_simulator_core/entities/assets/pyjnius_loader.py @@ -18,6 +18,7 @@ import hashlib import logging import os +import stat import tempfile import urllib.request from typing import Callable @@ -100,9 +101,10 @@ def download_rosim_jar() -> str: The JAR is downloaded at runtime rather than shipped inside the wheel because it exceeds the 100 MB per-file size limit on PyPI. - If a `rosim*.jar` is already present in the `bin` folder, no download happens and - that file is used. To pick up a newer Rosim JAR, bump the module-level constants; - deleting the local copy only re-fetches the same pinned asset. + If any `rosim*.jar` is already present in the `bin` folder, no download happens and + that file is used, whatever its version. Picking up a newer Rosim JAR therefore takes + both steps: bump the module-level constants and delete the local copy, since either + one alone leaves the existing file in place. The JAR is fetched from a pinned release asset URL, written to a temporary file in the `bin` folder and verified against `ROSIM_JAR_SHA256`. Only a file with a matching @@ -116,7 +118,7 @@ def download_rosim_jar() -> str: jar_files = glob.glob(os.path.join(bin_path, "rosim*.jar")) if jar_files: logger.debug("Rosim JAR files already present, skipping download.") - logger.debug(f"Using Rosim JAR file: {jar_files[0]}") + logger.debug("Using Rosim JAR file: %s", jar_files[0]) return os.path.basename(jar_files[0]) logger.debug("Downloading Rosim JAR files from GitHub") @@ -130,11 +132,13 @@ def download_rosim_jar() -> str: try: os.close(temp_fd) # mkstemp creates the file 0600 and os.replace preserves that, where a plain - # download would have taken the umask default. Re-apply the umask so the JAR is - # readable by whoever the JVM runs as, without widening past what the umask allows. - umask = os.umask(0) - os.umask(umask) - os.chmod(temp_path, 0o666 & ~umask) + # download would have taken the umask default. Copy the mode of a sibling that the + # package itself shipped, so the JAR ends up as readable as the rest of `bin` for + # whoever the JVM runs as. Reading the umask instead would mean mutating + # process-global state, which is not safe to do from a library. + reference = os.path.join(bin_path, "jfxrt.jar") + if os.path.exists(reference): + os.chmod(temp_path, stat.S_IMODE(os.stat(reference).st_mode)) urllib.request.urlretrieve(ROSIM_JAR_URL, temp_path) actual_hash = _sha256_of_file(temp_path) if actual_hash != ROSIM_JAR_SHA256: @@ -150,10 +154,10 @@ def download_rosim_jar() -> str: except OSError: # A failure to clean up must not replace the error that caused it, which # carries the reason the download is unusable. - logger.warning(f"Failed to remove temporary download {temp_path}.") + logger.warning("Failed to remove temporary download %s.", temp_path, exc_info=True) raise - logger.debug(f"Using Rosim JAR file: {final_path}") + logger.debug("Using Rosim JAR file: %s", final_path) return ROSIM_JAR_NAME def load_class(self, classpath: str) -> JavaClass: From c275a32eebcd15f5569dc5cc5c889f2ca6f59c81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jes=C3=BAs=20A=2E=20Rodr=C3=ADguez?= Date: Thu, 3 Sep 2026 12:53:15 +0200 Subject: [PATCH 3/3] Leave the downloaded JAR at mkstemp's mode Copying the mode from jfxrt.jar silently did nothing in the one case the neighbouring os.makedirs call exists for: a bin folder that is absent or empty holds no sibling to copy from. No OMOTES deployment writes the JAR as one user and reads it as another, so the mode is left alone and the reason recorded. --- .../entities/assets/pyjnius_loader.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/omotes_simulator_core/entities/assets/pyjnius_loader.py b/src/omotes_simulator_core/entities/assets/pyjnius_loader.py index e51c7604..2ce14a68 100644 --- a/src/omotes_simulator_core/entities/assets/pyjnius_loader.py +++ b/src/omotes_simulator_core/entities/assets/pyjnius_loader.py @@ -18,7 +18,6 @@ import hashlib import logging import os -import stat import tempfile import urllib.request from typing import Callable @@ -131,14 +130,11 @@ def download_rosim_jar() -> str: temp_fd, temp_path = tempfile.mkstemp(dir=bin_path, prefix=".rosim-download-") try: os.close(temp_fd) - # mkstemp creates the file 0600 and os.replace preserves that, where a plain - # download would have taken the umask default. Copy the mode of a sibling that the - # package itself shipped, so the JAR ends up as readable as the rest of `bin` for - # whoever the JVM runs as. Reading the umask instead would mean mutating - # process-global state, which is not safe to do from a library. - reference = os.path.join(bin_path, "jfxrt.jar") - if os.path.exists(reference): - os.chmod(temp_path, stat.S_IMODE(os.stat(reference).st_mode)) + # The JAR keeps mkstemp's 0600 rather than the umask default a plain download + # would have taken, which only matters if it is written by one user and read by + # another. No OMOTES deployment switches user between the two, so the mode is + # left alone: reading the umask to widen it would mean mutating process-global + # state from library code. urllib.request.urlretrieve(ROSIM_JAR_URL, temp_path) actual_hash = _sha256_of_file(temp_path) if actual_hash != ROSIM_JAR_SHA256: