diff --git a/CHANGELOG.md b/CHANGELOG.md index 48d3c2d5..2b10fe6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ All notable changes to the Zowe Client Python SDK will be documented in this fil ### Bug Fixes +- Fixed `Jobs.get_job_output_as_files` writing to a directory it never created, and made job output paths stay within the target directory. [#403](https://github.com/zowe/zowe-client-python-sdk/pull/403) - Updated the `pyo3` dependency of the Secrets SDK for technical currency. [#399](https://github.com/zowe/zowe-client-python-sdk/pull/399) ## `1.0.0-dev26` diff --git a/src/zos_files/zowe/zos_files_for_zowe_sdk/datasets.py b/src/zos_files/zowe/zos_files_for_zowe_sdk/datasets.py index f041bd59..8a5f95f8 100644 --- a/src/zos_files/zowe/zos_files_for_zowe_sdk/datasets.py +++ b/src/zos_files/zowe/zos_files_for_zowe_sdk/datasets.py @@ -577,10 +577,10 @@ def retrieve_content( ---------- dataset_name: str The name of the dataset - content_type: ContentType, optional + content_type: ContentType The content type to receive ("text", "binary" or "record" (include a 4 byte big endian record len prefix), "text" by default) - as_stream: bool, optional + as_stream: bool Specifies whether the response is streamed. Default: False Returns @@ -667,7 +667,7 @@ def perform_download( Name of the dataset to be downloaded local_file_path: str Name of the file to be saved locally - content_type: ContentType, optional + content_type: ContentType The content type to receive ("text", "binary" or "record" (include a 4 byte big endian record len prefix), "text" by default) @@ -719,10 +719,10 @@ def perform_upload( Name of the file to be uploaded dataset_name: str Name of the dataset to be created - content_type: ContentType, optional + content_type: ContentType The content type to receive ("text", "binary" or "record" (include a 4 byte big endian record len prefix), "text" by default) - upload_in_encoding: str, optional + upload_in_encoding: str Specifies the encoding to upload the content in (e.g. IBM-1047, "utf-8" by default) Raises diff --git a/src/zos_files/zowe/zos_files_for_zowe_sdk/uss.py b/src/zos_files/zowe/zos_files_for_zowe_sdk/uss.py index af014ed8..05ee29b2 100644 --- a/src/zos_files/zowe/zos_files_for_zowe_sdk/uss.py +++ b/src/zos_files/zowe/zos_files_for_zowe_sdk/uss.py @@ -149,13 +149,13 @@ def retrieve_content( ---------- file_path: str Path of the file - content_type: ContentType, optional + content_type: ContentType The content type to receive ("text" or "binary", "text" by default) - remote_file_encoding: str, optional + remote_file_encoding: str Encoding file content originally in (to convert from; by default, it is always being converted from "IBM-1047") - receive_in_encoding: str, optional + receive_in_encoding: str Encoding to convert file content to (to convert to; by default, it is always being converted to "ISO8859-1") - as_stream: bool, optional + as_stream: bool Specifies whether the response is streamed. Default: False Returns @@ -237,11 +237,11 @@ def perform_download( Path of the file to be downloaded local_file_path: str Name of the file to be saved locally - content_type: ContentType, optional + content_type: ContentType Specifies the content type to fetch ("text" or "binary", "text" by default) - remote_file_encoding: str, optional + remote_file_encoding: str Encoding file content originally in (to convert from; by default, it is always being converted from "IBM-1047") - receive_in_encoding: str, optional + receive_in_encoding: str Encoding to convert file content to (to convert to; by default, it is always being converted to "UTF-8" during download). Ignored when "binary" is True @@ -311,7 +311,7 @@ def perform_upload( Name of the file to be uploaded remote_file_path: str Path of the file where it will be created - content_type: ContentType, optional + content_type: ContentType Specifies the content type to fetch ("text" or "binary", "text" by default) upload_in_encoding: str Specifies encoding schema of the uploaded file diff --git a/src/zos_jobs/zowe/zos_jobs_for_zowe_sdk/jobs.py b/src/zos_jobs/zowe/zos_jobs_for_zowe_sdk/jobs.py index 81d3481c..94042521 100644 --- a/src/zos_jobs/zowe/zos_jobs_for_zowe_sdk/jobs.py +++ b/src/zos_jobs/zowe/zos_jobs_for_zowe_sdk/jobs.py @@ -440,6 +440,32 @@ def get_spool_file_contents(self, correlator: str, id: str) -> str: response_json: str = self.request_handler.perform_request("GET", custom_args) return response_json + @classmethod + def _reject_unsafe_component(cls, component: str) -> None: + # A JES-supplied name must be a single segment, never a ".." step or an absolute path + normalized = str(component).replace("\\", "/") + if os.path.isabs(component) or ".." in normalized.split("/"): + raise ValueError("Invalid job output path component: {}".format(component)) + + @staticmethod + def _is_sub_path(parent: str, child: str) -> bool: + # True only when child resolves to a location inside parent, mirrors IO.isSubPath in the Node SDK + try: + relative_path = os.path.relpath(os.path.realpath(child), os.path.realpath(parent)) + except ValueError: + # Raised on Windows when the paths live on different drives + return False + segments = relative_path.split(os.sep) if relative_path else [] + if not segments or ".." in segments or os.path.isabs(relative_path): + return False + return True + + @classmethod + def _reject_unsafe_path(cls, base_dir: str, target: str) -> None: + # Final guard that the generated path stays inside base_dir before any file is written + if not cls._is_sub_path(base_dir, target): + raise ValueError("The generated file path is outside the output directory: {}".format(target)) + def get_job_output_as_files(self, status: dict[str, Any], output_dir: str) -> None: """ Get all spool files and submitted jcl text in separate files in the specified output directory. @@ -469,9 +495,16 @@ def get_job_output_as_files(self, status: dict[str, Any], output_dir: str) -> No job_id = status["jobid"] job_correlator = status["job-correlator"] - output_dir = os.path.join(output_dir, job_name, job_id) - os.makedirs(output_dir, exist_ok=True) - output_file = os.path.join(output_dir, job_name, job_id, "jcl.txt") + # Fixed root that every generated path must stay within + base_dir = os.path.realpath(output_dir) + + self._reject_unsafe_component(job_name) + self._reject_unsafe_component(job_id) + job_dir = os.path.join(output_dir, job_name, job_id) + self._reject_unsafe_path(base_dir, job_dir) + os.makedirs(job_dir, exist_ok=True) + output_file = os.path.join(job_dir, "jcl.txt") + self._reject_unsafe_path(base_dir, output_file) data_spool_file = self.get_jcl_text(job_correlator) dataset_content = data_spool_file with open(output_file, "w", encoding="utf-8") as out_file: @@ -482,10 +515,14 @@ def get_job_output_as_files(self, status: dict[str, Any], output_dir: str) -> No stepname = spool_file["stepname"] ddname = spool_file["ddname"] spoolfile_id = spool_file["id"] - output_dir = os.path.join(output_dir, job_name, job_id, stepname) - os.makedirs(output_dir, exist_ok=True) - - output_file = os.path.join(output_dir, job_name, job_id, stepname, ddname) + self._reject_unsafe_component(stepname) + self._reject_unsafe_component(ddname) + step_dir = os.path.join(job_dir, stepname) + self._reject_unsafe_path(base_dir, step_dir) + os.makedirs(step_dir, exist_ok=True) + + output_file = os.path.join(step_dir, ddname) + self._reject_unsafe_path(base_dir, output_file) data_spool_file = self.get_spool_file_contents(job_correlator, spoolfile_id) dataset_content = data_spool_file with open(output_file, "w", encoding="utf-8") as out_file: diff --git a/tests/unit/test_zos_jobs.py b/tests/unit/test_zos_jobs.py index 44794c65..3a018e37 100644 --- a/tests/unit/test_zos_jobs.py +++ b/tests/unit/test_zos_jobs.py @@ -1,5 +1,8 @@ """Unit tests for the Zowe Python SDK z/OS Jobs package.""" +import os +import shutil +import tempfile from unittest import TestCase, mock from zowe.zos_jobs_for_zowe_sdk import Jobs @@ -134,3 +137,61 @@ def test_cancel_job_modify_version_parameterized(self): with self.assertRaises(ValueError) as e_info: jobs_test_object.cancel_job(*test_case[0]) self.assertEqual(str(e_info.exception), 'Accepted values for modify_version: "1.0" or "2.0"') + + def _mock_jobs_for_output(self, spool_files): + """Build a Jobs object with the spool-fetching methods stubbed out.""" + jobs = Jobs(self.test_profile) + jobs.get_jcl_text = mock.Mock(return_value="//JOBCARD JOB\n") + jobs.get_spool_files = mock.Mock(return_value=spool_files) + jobs.get_spool_file_contents = mock.Mock(side_effect=lambda c, sid: "content-{}\n".format(sid)) + return jobs + + def test_get_job_output_as_files_writes_expected_files(self): + """Spool files and jcl.txt are written under output_dir/jobname/jobid.""" + out_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, out_dir, ignore_errors=True) + jobs = self._mock_jobs_for_output([{"stepname": "STEP1", "ddname": "JESMSGLG", "id": "1"}]) + status = {"jobname": "MYJOB", "jobid": "JOB001", "job-correlator": "C1"} + + jobs.get_job_output_as_files(status, out_dir) + + jcl_file = os.path.join(out_dir, "MYJOB", "JOB001", "jcl.txt") + spool_file = os.path.join(out_dir, "MYJOB", "JOB001", "STEP1", "JESMSGLG") + self.assertTrue(os.path.isfile(jcl_file)) + self.assertTrue(os.path.isfile(spool_file)) + with open(spool_file, encoding="utf-8") as f: + self.assertEqual(f.read(), "content-1\n") + + def test_get_job_output_as_files_rejects_absolute_component(self): + """An absolute jobid must not escape output_dir and raises ValueError.""" + out_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, out_dir, ignore_errors=True) + jobs = self._mock_jobs_for_output([]) + status = {"jobname": "MYJOB", "jobid": os.path.join(os.sep, "tmp", "pwn"), "job-correlator": "C2"} + + with self.assertRaises(ValueError): + jobs.get_job_output_as_files(status, out_dir) + + def test_get_job_output_as_files_rejects_backtrack_component(self): + """A stepname containing '..' must not escape output_dir and raises ValueError.""" + out_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, out_dir, ignore_errors=True) + escaped = os.path.join("..", "..", "evil") + jobs = self._mock_jobs_for_output([{"stepname": escaped, "ddname": "X", "id": "9"}]) + status = {"jobname": "MYJOB", "jobid": "JOB003", "job-correlator": "C3"} + + with self.assertRaises(ValueError): + jobs.get_job_output_as_files(status, out_dir) + self.assertFalse(os.path.exists(os.path.join(out_dir, "..", "..", "evil", "X"))) + + def test_get_job_output_as_files_rejects_absolute_child(self): + """An absolute leaf ddname is not honored even though join would drop the parent.""" + out_dir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, out_dir, ignore_errors=True) + abs_ddname = os.path.join(os.sep, "etc", "cron.d", "evil") + jobs = self._mock_jobs_for_output([{"stepname": "STEP1", "ddname": abs_ddname, "id": "7"}]) + status = {"jobname": "MYJOB", "jobid": "JOB004", "job-correlator": "C4"} + + with self.assertRaises(ValueError): + jobs.get_job_output_as_files(status, out_dir) + self.assertFalse(os.path.exists(abs_ddname))