From ccc814f1844edc011571742206ab10a7135203da Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Thu, 3 Sep 2026 10:27:52 -0600 Subject: [PATCH 1/5] harden Slurm release artifacts Add sanitized credential and infrastructure scans for source, fixtures, runtime archives, logs, and wheels. Include Apache license text in the Slurm wheel and redact sensitive scheduler diagnostics. Signed-off-by: Nabin Mulepati --- packages/data-designer-slurm/LICENSE | 201 ++++++++++ packages/data-designer-slurm/pyproject.toml | 1 + .../data_designer/slurm/launcher/client.py | 4 +- .../src/data_designer/slurm/runtime/bundle.py | 3 + .../src/data_designer/slurm/security.py | 47 +++ .../tests/launcher/test_client.py | 28 ++ .../tests/runtime/test_bundle.py | 4 +- .../data-designer-slurm/tests/test_package.py | 6 + .../tests/test_public_artifacts.py | 145 ++++++++ plans/870/slurm-early-security-review.md | 60 +++ scripts/audit_slurm_public_artifacts.py | 345 ++++++++++++++++++ scripts/test_slurm_package_install.py | 11 + 12 files changed, 853 insertions(+), 2 deletions(-) create mode 100644 packages/data-designer-slurm/LICENSE create mode 100644 packages/data-designer-slurm/src/data_designer/slurm/security.py create mode 100644 packages/data-designer-slurm/tests/test_public_artifacts.py create mode 100644 plans/870/slurm-early-security-review.md create mode 100644 scripts/audit_slurm_public_artifacts.py diff --git a/packages/data-designer-slurm/LICENSE b/packages/data-designer-slurm/LICENSE new file mode 100644 index 000000000..d67d6cffa --- /dev/null +++ b/packages/data-designer-slurm/LICENSE @@ -0,0 +1,201 @@ + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright 2025 NVIDIA + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. diff --git a/packages/data-designer-slurm/pyproject.toml b/packages/data-designer-slurm/pyproject.toml index 9e4610eff..76c86faa8 100644 --- a/packages/data-designer-slurm/pyproject.toml +++ b/packages/data-designer-slurm/pyproject.toml @@ -5,6 +5,7 @@ description = "Slurm execution for Data Designer" readme = "README.md" requires-python = ">=3.10" license = "Apache-2.0" +license-files = ["LICENSE"] classifiers = [ "Development Status :: 4 - Beta", diff --git a/packages/data-designer-slurm/src/data_designer/slurm/launcher/client.py b/packages/data-designer-slurm/src/data_designer/slurm/launcher/client.py index 47d061ad1..3648e30de 100644 --- a/packages/data-designer-slurm/src/data_designer/slurm/launcher/client.py +++ b/packages/data-designer-slurm/src/data_designer/slurm/launcher/client.py @@ -28,6 +28,7 @@ parse_submission, ) from data_designer.slurm.launcher.runner import CommandRunner, SubprocessRunner +from data_designer.slurm.security import redact_sensitive_text from data_designer.slurm.state import SchedulerIdentity _JobSelector: TypeAlias = int | SchedulerIdentity @@ -218,7 +219,8 @@ def _validate_argument(value: str, *, field_name: str) -> None: def _normalize_bounded_text(value: str, *, limit: int = 512) -> str: sanitized = "".join(" " if unicodedata.category(character).startswith("C") else character for character in value) normalized = " ".join(sanitized.split()) - return normalized if len(normalized) <= limit else f"{normalized[: limit - 3]}..." + redacted = redact_sensitive_text(normalized) + return redacted if len(redacted) <= limit else f"{redacted[: limit - 3]}..." def _format_error_detail(error: BaseException) -> str: diff --git a/packages/data-designer-slurm/src/data_designer/slurm/runtime/bundle.py b/packages/data-designer-slurm/src/data_designer/slurm/runtime/bundle.py index 7027c496c..2b55d293e 100644 --- a/packages/data-designer-slurm/src/data_designer/slurm/runtime/bundle.py +++ b/packages/data-designer-slurm/src/data_designer/slurm/runtime/bundle.py @@ -29,6 +29,9 @@ _SLURM_PACKAGE_ROOT = "data_designer/slurm" _SOURCE_MANIFEST_NAME = f"{_SLURM_PACKAGE_ROOT}/runtime/slurm-sources.txt" _ENTRYPOINT = b"""#!/usr/bin/env bash +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + set -Eeuo pipefail dd_slurm_run_allocation() { diff --git a/packages/data-designer-slurm/src/data_designer/slurm/security.py b/packages/data-designer-slurm/src/data_designer/slurm/security.py new file mode 100644 index 000000000..4901d7f52 --- /dev/null +++ b/packages/data-designer-slurm/src/data_designer/slurm/security.py @@ -0,0 +1,47 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Security helpers shared by Slurm process boundaries.""" + +from __future__ import annotations + +import re + +from data_designer.slurm.config.environment import is_secret_bearing_name + +_ASSIGNMENT_PATTERN = re.compile( + r"(?P(?P[\"']?)(?P-{0,2}[A-Za-z][A-Za-z0-9_.-]*)" + r"(?P=quote)\s*[:=]\s*)(?P\"[^\"]*\"|'[^']*'|[^\s,;]+)" +) +_OPTION_PATTERN = re.compile(r"(?P(?P--[A-Za-z][A-Za-z0-9.-]*)\s+)(?P\"[^\"]*\"|'[^']*'|[^\s]+)") +_AUTHORIZATION_PATTERN = re.compile(r"(?i)(?P\bauthorization\s*[:=]\s*(?:basic|bearer)\s+)(?P[^\s,;]+)") +_URL_USERINFO_PATTERN = re.compile(r"(?i)(?P\bhttps?://)[^/@\s]+@") +_TOKEN_PATTERNS = ( + re.compile(r"\bAKIA[0-9A-Z]{16}\b"), + re.compile(r"\bgithub_pat_[A-Za-z0-9_]{20,}\b"), + re.compile(r"\bgh[pousr]_[A-Za-z0-9_]{20,}\b"), + re.compile(r"\bnvapi-[A-Za-z0-9_-]{20,}\b"), + re.compile(r"\bsk-(?:proj-)?[A-Za-z0-9_-]{20,}\b"), +) +_REDACTION = "" + + +def redact_sensitive_text(value: str) -> str: + """Redact recognizable credentials without echoing their values.""" + redacted = _AUTHORIZATION_PATTERN.sub(lambda match: f"{match.group('prefix')}{_REDACTION}", value) + redacted = _URL_USERINFO_PATTERN.sub(lambda match: f"{match.group('scheme')}{_REDACTION}@", redacted) + redacted = _ASSIGNMENT_PATTERN.sub(_redact_named_value, redacted) + redacted = _OPTION_PATTERN.sub(_redact_named_value, redacted) + for pattern in _TOKEN_PATTERNS: + redacted = pattern.sub(_REDACTION, redacted) + return redacted + + +def _redact_named_value(match: re.Match[str]) -> str: + name = match.group("name").lstrip("-") + if not is_secret_bearing_name(name): + return match.group(0) + return f"{match.group('prefix')}{_REDACTION}" + + +__all__ = ["redact_sensitive_text"] diff --git a/packages/data-designer-slurm/tests/launcher/test_client.py b/packages/data-designer-slurm/tests/launcher/test_client.py index 183680d34..4246dff0d 100644 --- a/packages/data-designer-slurm/tests/launcher/test_client.py +++ b/packages/data-designer-slurm/tests/launcher/test_client.py @@ -189,6 +189,34 @@ def test_client_removes_terminal_controls_from_command_failures(fake_slurm_runne assert "\x1b" not in str(error.value) +@pytest.mark.parametrize( + ("diagnostic", "secret"), + ( + ("HF_TOKEN=super-secret-value", "super-secret-value"), + ('HF_TOKEN="quoted secret value"', "quoted secret value"), + ("--api-key plaintext-secret", "plaintext-secret"), + ('{"access_token":"json-secret"}', "json-secret"), + ("Authorization: Bearer bearer-secret", "bearer-secret"), + ("https://user:url-secret@example.test/index", "url-secret"), + (f"token github_pat_{'a' * 24}", f"github_pat_{'a' * 24}"), + ), + ids=("environment", "quoted-environment", "option", "json", "authorization", "url", "known-token"), +) +def test_client_redacts_secrets_from_command_failures( + fake_slurm_runner: FakeSlurmRunner, + diagnostic: str, + secret: str, +) -> None: + fake_slurm_runner.script_next("squeue", FakeCommandResponse(stderr=diagnostic, returncode=2)) + client = SlurmCommandClient(fake_slurm_runner) + + with pytest.raises(SlurmCommandError) as error: + client.query_queue((4101,)) + + assert secret not in str(error.value) + assert "" in str(error.value) + + def test_client_bounds_command_failure_detail(fake_slurm_runner: FakeSlurmRunner) -> None: fake_slurm_runner.script_next("squeue", FakeCommandResponse(stderr="x" * 600, returncode=2)) client = SlurmCommandClient(fake_slurm_runner) diff --git a/packages/data-designer-slurm/tests/runtime/test_bundle.py b/packages/data-designer-slurm/tests/runtime/test_bundle.py index a1d67bde8..a22c78aa5 100644 --- a/packages/data-designer-slurm/tests/runtime/test_bundle.py +++ b/packages/data-designer-slurm/tests/runtime/test_bundle.py @@ -46,7 +46,9 @@ def test_runtime_bundle_is_deterministic_content_addressed_and_restrictive(tmp_p assert all(archive.getmember(name).uid == 0 for name in names) entrypoint = archive.extractfile("entrypoint.sh") assert entrypoint is not None - assert b'PYTHONPATH="${runtime_root}"' in entrypoint.read() + entrypoint_content = entrypoint.read() + assert b"SPDX-License-Identifier: Apache-2.0" in entrypoint_content + assert b'PYTHONPATH="${runtime_root}"' in entrypoint_content def test_runtime_bundle_recursively_collects_and_imports_nested_packages( diff --git a/packages/data-designer-slurm/tests/test_package.py b/packages/data-designer-slurm/tests/test_package.py index 2544dd973..24a4b4955 100644 --- a/packages/data-designer-slurm/tests/test_package.py +++ b/packages/data-designer-slurm/tests/test_package.py @@ -17,6 +17,12 @@ def test_slurm_uses_shared_namespace() -> None: assert data_designer.slurm.__name__ == "data_designer.slurm" +def test_slurm_license_matches_repository() -> None: + package_license = REPO_ROOT / "packages" / "data-designer-slurm" / "LICENSE" + + assert package_license.read_bytes() == (REPO_ROOT / "LICENSE").read_bytes() + + def test_slurm_is_published_before_base_extra() -> None: publish_script = (REPO_ROOT / "scripts" / "publish.sh").read_text() diff --git a/packages/data-designer-slurm/tests/test_public_artifacts.py b/packages/data-designer-slurm/tests/test_public_artifacts.py new file mode 100644 index 000000000..d0abae0e3 --- /dev/null +++ b/packages/data-designer-slurm/tests/test_public_artifacts.py @@ -0,0 +1,145 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +from __future__ import annotations + +import subprocess +import sys +import tarfile +import zipfile +from io import BytesIO +from pathlib import Path + +import pytest + +REPOSITORY_ROOT = Path(__file__).parents[3] +AUDIT_SCRIPT = REPOSITORY_ROOT / "scripts" / "audit_slurm_public_artifacts.py" +SPDX_HEADER = """# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +""" + + +def test_repository_slurm_artifacts_pass_public_audit() -> None: + result = _run_audit() + + assert result.returncode == 0, result.stderr + + +@pytest.mark.parametrize( + ("content", "rule"), + ( + (f"token=github_pat_{'a' * 24}\n", "GitHub token"), + ("workspace=/home/specific-user/run\n", "user-specific absolute path"), + ("endpoint=10.23.45.67\n", "private infrastructure address"), + ("host=service.internal.nvidia.com\n", "internal NVIDIA hostname"), + ), + ids=("credential", "user-path", "private-address", "internal-host"), +) +def test_public_audit_reports_rule_without_echoing_sensitive_content( + tmp_path: Path, + content: str, + rule: str, +) -> None: + artifact = tmp_path / "runtime.log" + artifact.write_text(content) + + result = _run_audit(artifact) + + assert result.returncode == 1 + assert rule in result.stderr + assert content.strip() not in result.stderr + assert str(tmp_path) not in result.stderr + + +def test_public_audit_checks_wheel_members_and_license_text(tmp_path: Path) -> None: + wheel = tmp_path / "data_designer_slurm-1.0.0-py3-none-any.whl" + with zipfile.ZipFile(wheel, mode="w") as archive: + archive.writestr("data_designer/slurm/runtime.py", f"{SPDX_HEADER}\nfrom __future__ import annotations\n") + archive.writestr( + "data_designer_slurm-1.0.0.dist-info/licenses/LICENSE", + "Apache License\nVersion 2.0\n", + ) + archive.writestr( + "data_designer_slurm-1.0.0.dist-info/METADATA", + "Metadata-Version: 2.5\nLicense-Expression: Apache-2.0\n", + ) + + result = _run_audit(wheel) + + assert result.returncode == 0, result.stderr + + +def test_public_audit_rejects_unsafe_or_unlicensed_wheel_members(tmp_path: Path) -> None: + wheel = tmp_path / "data_designer_slurm-1.0.0-py3-none-any.whl" + with zipfile.ZipFile(wheel, mode="w") as archive: + archive.writestr("../escaped.py", "pass\n") + archive.writestr("data_designer/slurm/runtime.py", "from __future__ import annotations\n") + + result = _run_audit(wheel) + + assert result.returncode == 1 + assert "archive member path is unsafe" in result.stderr + assert "missing the NVIDIA Apache-2.0 SPDX header" in result.stderr + assert "wheel does not contain its declared license text" in result.stderr + assert "wheel metadata does not declare Apache-2.0" in result.stderr + + +def test_public_audit_scans_archive_member_names_without_echoing_them(tmp_path: Path) -> None: + wheel = tmp_path / "data_designer_slurm-1.0.0-py3-none-any.whl" + sensitive_member = "data_designer/slurm/10.23.45.67.py" + with zipfile.ZipFile(wheel, mode="w") as archive: + archive.writestr(sensitive_member, SPDX_HEADER) + archive.writestr( + "data_designer_slurm-1.0.0.dist-info/licenses/LICENSE", + "Apache License\nVersion 2.0\n", + ) + archive.writestr( + "data_designer_slurm-1.0.0.dist-info/METADATA", + "Metadata-Version: 2.5\nLicense-Expression: Apache-2.0\n", + ) + + result = _run_audit(wheel) + + assert result.returncode == 1 + assert "private infrastructure address" in result.stderr + assert sensitive_member not in result.stderr + + +def test_public_audit_checks_runtime_tar_content_and_entrypoint_license(tmp_path: Path) -> None: + archive_path = tmp_path / "runtime.tar.gz" + secret = f"nvapi-{'a' * 24}" + with tarfile.open(archive_path, mode="w:gz") as archive: + entrypoint = tarfile.TarInfo("entrypoint.sh") + entrypoint_content = f"#!/bin/sh\necho {secret}\n".encode() + entrypoint.size = len(entrypoint_content) + archive.addfile(entrypoint, BytesIO(entrypoint_content)) + + result = _run_audit(archive_path) + + assert result.returncode == 1 + assert "NGC API key" in result.stderr + assert "missing the NVIDIA Apache-2.0 SPDX header" in result.stderr + assert secret not in result.stderr + + +def test_public_audit_rejects_explicit_symbolic_link_without_disclosing_its_parent(tmp_path: Path) -> None: + target = tmp_path / "target.log" + target.write_text("safe\n") + link = tmp_path / "linked.log" + link.symlink_to(target) + + result = _run_audit(link) + + assert result.returncode == 1 + assert "symbolic-link artifact requires explicit review" in result.stderr + assert str(tmp_path) not in result.stderr + + +def _run_audit(*paths: Path) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [sys.executable, str(AUDIT_SCRIPT), *(str(path) for path in paths)], + cwd=REPOSITORY_ROOT, + check=False, + capture_output=True, + text=True, + ) diff --git a/plans/870/slurm-early-security-review.md b/plans/870/slurm-early-security-review.md new file mode 100644 index 000000000..02b455b8d --- /dev/null +++ b/plans/870/slurm-early-security-review.md @@ -0,0 +1,60 @@ +# Slurm early security and provenance review + +This document records the dependency-ready portion of #870. It reviews the public Slurm implementation through +the one-node runtime merge and the hardening changes developed with this review. It is not final release acceptance: +the sealed commit, complete wheel set, runtime checksum, real-cluster scenarios, sanitized profile rerun, and all +dependent implementation and documentation must still be frozen and validated together. + +## Threat model boundaries + +| Boundary | Threats reviewed | Existing or added controls | Evidence | +| --- | --- | --- | --- | +| Slurm submission | Shell injection, option confusion, inherited secrets, unbounded caller-visible diagnostics | Commands use argument vectors without a shell; batch options reject controls; script paths reject option-like names; the launcher forwards only an explicit environment; diagnostic text is normalized, redacted, and limited to 512 characters | `tests/launcher/test_client.py`, `tests/launcher/test_renderer.py`, `tests/launcher/test_runner.py` | +| Batch entrypoint | Directive splitting, shell expansion, plan or runtime substitution, unsafe task identity | Directive names and values are validated; shell values are escaped; the script fixes `PATH`, checks both SHA-256 identities before extraction, validates the array-task ID, and creates a private attempt-local runtime directory | `tests/launcher/test_renderer.py`, `tests/slurm_test_fakes/test_rendered_scripts.py` | +| Runtime commands and environment | Shell fragments, ambient environment leakage, persisted plaintext credentials | Runtime steps are immutable argument vectors; `Popen` uses `shell=False`; only package-owned scheduler variables and explicitly resolved bindings are forwarded; secret-shaped values require environment references and are not persisted | `tests/contracts/test_config_records.py`, `tests/runtime/test_steps.py`, `tests/runtime/test_supervisor.py` | +| Host and container paths | Parent traversal, ambiguous paths, mount escape, read-only mount writes | Persisted paths are normalized absolute POSIX paths below `/`; container translation selects the most-specific resolved mount and separately enforces write access; state and logs use descriptor-bound, no-follow operations | `tests/runtime/test_paths.py`, `tests/runtime/test_preflight.py`, `tests/state/test_store.py` | +| Images and runtime archives | Credential-bearing image references, archive traversal, replacement races, altered runtime source | OCI sources reject credentials and ambiguous schemes; image/state publication is restrictive and atomic; the runtime archive is package-built with fixed member names and metadata, content-addressed, and verified before extraction | `tests/images/test_lifecycle.py`, `tests/images/test_registry_store.py`, `tests/runtime/test_bundle.py` | +| Logs and public evidence | Secret or site-specific data copied into public artifacts | Runtime logs are private `0600` files below private execution directories. The public-artifact audit reports only a display path and rule name, scans explicit log paths without echoing matches, and rejects high-confidence credentials and environment-specific infrastructure values | `tests/runtime/test_supervisor.py`, `tests/test_public_artifacts.py` | +| Cleanup and signals | Orphaned process groups, repeated cleanup, partial publication | Runtime children start in owned sessions; cleanup is idempotent, terminates in reverse order, escalates after a bounded grace period, and surfaces normalized failure state | `tests/runtime/test_supervisor.py`, `tests/runtime/test_controller.py` | + +## Findings resolved in this slice + +- The Slurm wheel declared Apache-2.0 but did not carry the license text. The package now includes a canonical copy of + the repository license, verifies the copies are byte-identical, and fails the wheel audit unless the expanded wheel + contains Apache License 2.0 text. +- Slurm command failures bounded and control-normalized scheduler stderr but did not explicitly redact recognizable + credentials. A shared redaction helper now covers secret-shaped assignments and options, authorization headers, URL + user information, and high-confidence provider token formats before diagnostic truncation. +- Public-artifact checks were limited to individual golden tests. The new scanner covers deployable source, package + metadata, public fixtures, examples/documentation, release scripts, explicit logs, ZIP/wheel members, and tar members. + Archive traversal, links, excessive member counts, excessive expanded content, missing packaged-source SPDX headers, + and missing wheel license text fail closed. + +## Provenance and dependency review + +- All reviewed package Python and shell resources carry NVIDIA Apache-2.0 SPDX headers. Package history identifies the + resources as repository contributions; no copied or adapted third-party source was identified in this scope. +- Direct runtime dependencies remain `data-designer`, `packaging`, `pydantic`, and `pyyaml`. The dependency inventory + reports only `click` and `typer` as transitive imports guaranteed by the exact-version `data-designer` dependency; + there are no unresolved imported modules. +- The scanner deliberately permits only generic test representations that its high-confidence rules do not classify: + `example.test` hosts, loopback addresses, and `/workspace` paths. Python test modules contain deliberate credential + sentinels and are not default publication inputs; maintained golden and fixture artifacts are scanned. + +## Commands for this review slice + +```bash +python scripts/audit_slurm_public_artifacts.py +python scripts/audit_slurm_public_artifacts.py path/to/wheel.whl path/to/sanitized.log +make test-slurm +make test-slurm-wheel-install +make check-slurm +make check-dependency-licenses +``` + +## Remaining final acceptance + +After all #850 implementation and documentation dependencies are merged, #870 must select one public commit and build +the complete wheel set once. The source, wheels, runtime archive, sanitized scenario evidence, and sanitized profile +rerun must all be tied to those exact digests. Any code or wheel change after sealing invalidates the affected evidence. +Environment-specific profiles and raw scheduler or allocation logs remain outside public artifacts and issue comments. diff --git a/scripts/audit_slurm_public_artifacts.py b/scripts/audit_slurm_public_artifacts.py new file mode 100644 index 000000000..87b88dfd2 --- /dev/null +++ b/scripts/audit_slurm_public_artifacts.py @@ -0,0 +1,345 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +"""Scan public Slurm artifacts without rendering matched sensitive content. + +The default scope covers deployable source, package metadata, maintained public +fixtures, and release scripts. Python test modules are intentionally outside that +scope because they contain synthetic credential sentinels; their golden and fixture +artifacts remain in scope. Generic ``example.test`` hosts, loopback addresses, and +``/workspace`` paths are the only implicit test-data allowances. +""" + +from __future__ import annotations + +import argparse +import os +import re +import stat +import sys +import tarfile +import zipfile +from dataclasses import dataclass +from pathlib import Path, PurePosixPath +from typing import BinaryIO, Iterable + +MAXIMUM_MEMBER_SIZE = 16 * 1024 * 1024 +MAXIMUM_ARCHIVE_CONTENT_SIZE = 128 * 1024 * 1024 +MAXIMUM_ARCHIVE_MEMBERS = 10_000 +REPOSITORY_ROOT = Path(__file__).parents[1] +DEFAULT_ARTIFACTS = ( + "packages/data-designer-slurm/src", + "packages/data-designer-slurm/README.md", + "packages/data-designer-slurm/pyproject.toml", + "packages/data-designer-slurm/LICENSE", + "packages/data-designer-slurm/tests/contracts/golden", + "packages/data-designer-slurm/tests/fixtures", + "packages/data-designer-slurm/tests/integration/golden", + "packages/data-designer-slurm/tests/serving/golden", + "packages/data-designer-slurm/tests/slurm_test_fakes/golden", + "packages/data-designer-slurm/tests/state/golden", + "plans/850/data-designer-contract.md", + "plans/870/slurm-early-security-review.md", + "scripts/test_slurm_package_install.py", +) +_TEXT_SUFFIXES = frozenset( + { + ".cfg", + ".err", + ".ini", + ".json", + ".log", + ".md", + ".out", + ".py", + ".rc", + ".sbatch", + ".sh", + ".toml", + ".txt", + ".yaml", + ".yml", + } +) +_LICENSED_SOURCE_SUFFIXES = frozenset({".py", ".rc", ".sh"}) +_SPDX_COPYRIGHT = "SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved." +_SPDX_LICENSE = "SPDX-License-Identifier: Apache-2.0" +_SLURM_DIST_INFO_PATTERN = re.compile(r"^data_designer_slurm-[^/]+\.dist-info/") + + +@dataclass(frozen=True, slots=True) +class AuditRule: + """One high-confidence public-artifact rejection rule.""" + + name: str + pattern: re.Pattern[str] + + +@dataclass(frozen=True, slots=True) +class AuditFinding: + """One path-scoped finding that intentionally omits matched content.""" + + location: str + rule: str + + +_CONTENT_RULES = ( + AuditRule("private key material", re.compile(r"-----BEGIN (?:[A-Z0-9 ]+ )?PRIVATE KEY-----")), + AuditRule("AWS access key", re.compile(r"\bAKIA[0-9A-Z]{16}\b")), + AuditRule("GitHub token", re.compile(r"\b(?:github_pat_|gh[pousr]_)[A-Za-z0-9_]{20,}\b")), + AuditRule("NGC API key", re.compile(r"\bnvapi-[A-Za-z0-9_-]{20,}\b")), + AuditRule("OpenAI API key", re.compile(r"\bsk-(?:proj-)?[A-Za-z0-9_-]{20,}\b")), + AuditRule( + "plaintext secret assignment", + re.compile( + r"(?i)\b(?:api[_-]?key|access[_-]?token|auth[_-]?token|password|secret|credential)" + r"\b\s*[:=]\s*[\"']?(?!<|\$|\{|\[)[A-Za-z0-9+/_.:@-]{16,}" + ), + ), + AuditRule("authorization credential", re.compile(r"(?i)\bauthorization\s*:\s*(?:basic|bearer)\s+\S{16,}")), + AuditRule( + "internal NVIDIA hostname", + re.compile(r"(?i)\b(?:(?:[a-z0-9-]+\.)*(?:corp|internal)|gitlab-master|urm)\.nvidia\.com\b"), + ), + AuditRule("user-specific absolute path", re.compile(r"/(?:Users|home|users)/[A-Za-z0-9._-]+(?:/|\b)")), + AuditRule("site-specific shared path", re.compile(r"/(?:lustre|gpfs|fsx|net)/[^\s\"']+", re.IGNORECASE)), + AuditRule( + "private infrastructure address", + re.compile(r"\b(?:10(?:\.\d{1,3}){3}|172\.(?:1[6-9]|2\d|3[01])(?:\.\d{1,3}){2}|192\.168(?:\.\d{1,3}){2})\b"), + ), +) + + +def audit_public_artifacts(paths: Iterable[Path]) -> tuple[AuditFinding, ...]: + """Return sanitized findings from files, directories, wheels, and tar archives.""" + findings: list[AuditFinding] = [] + for path in sorted((Path(os.path.abspath(candidate)) for candidate in paths), key=str): + if path.is_symlink(): + findings.append(AuditFinding(_display_path(path), "symbolic-link artifact requires explicit review")) + elif not path.exists(): + findings.append(AuditFinding(_display_path(path), "artifact does not exist")) + elif path.is_dir(): + for child in sorted(path.rglob("*")): + if child.is_symlink(): + findings.append( + AuditFinding(_display_path(child), "symbolic-link artifact requires explicit review") + ) + elif child.is_file(): + findings.extend(_audit_file(child)) + else: + findings.extend(_audit_file(path)) + return tuple(findings) + + +def _audit_file(path: Path) -> list[AuditFinding]: + if _is_zip_archive(path): + return _audit_zip(path) + if _is_tar_archive(path): + return _audit_tar(path) + if path.suffix.casefold() not in _TEXT_SUFFIXES: + return [] + try: + with path.open("rb") as stream: + content = _read_bounded(stream, expected_size=path.stat().st_size) + except OSError: + return [AuditFinding(_display_path(path), "artifact cannot be read")] + except ValueError as error: + return [AuditFinding(_display_path(path), str(error))] + location = _display_path(path) + return _audit_content(location, location, content) + + +def _audit_zip(path: Path) -> list[AuditFinding]: + findings: list[AuditFinding] = [] + license_present = False + license_declared = False + try: + with zipfile.ZipFile(path) as archive: + members = archive.infolist() + limit_finding = _get_archive_limit_finding( + path, + member_count=len(members), + content_size=sum(max(member.file_size, 0) for member in members), + ) + if limit_finding is not None: + return [limit_finding] + for index, member in enumerate(sorted(members, key=lambda item: item.filename), start=1): + location = f"{_display_path(path)}!member-{index}" + member_findings, contains_license, declares_license = _audit_zip_member(archive, member, location) + findings.extend(member_findings) + license_present = license_present or contains_license + license_declared = license_declared or declares_license + except (OSError, zipfile.BadZipFile): + return [AuditFinding(_display_path(path), "artifact is not a readable ZIP archive")] + if path.suffix.casefold() == ".whl" and not license_present: + findings.append(AuditFinding(_display_path(path), "wheel does not contain its declared license text")) + if path.suffix.casefold() == ".whl" and not license_declared: + findings.append(AuditFinding(_display_path(path), "wheel metadata does not declare Apache-2.0")) + return findings + + +def _audit_tar(path: Path) -> list[AuditFinding]: + findings: list[AuditFinding] = [] + content_size = 0 + try: + with tarfile.open(path, mode="r:*") as archive: + for member_count, member in enumerate(archive, start=1): + content_size += max(member.size, 0) + limit_finding = _get_archive_limit_finding( + path, + member_count=member_count, + content_size=content_size, + ) + if limit_finding is not None: + return [limit_finding] + location = f"{_display_path(path)}!member-{member_count}" + findings.extend(_audit_tar_member(archive, member, location)) + except (OSError, tarfile.TarError): + return [AuditFinding(_display_path(path), "artifact is not a readable tar archive")] + return findings + + +def _audit_zip_member( + archive: zipfile.ZipFile, + member: zipfile.ZipInfo, + location: str, +) -> tuple[list[AuditFinding], bool, bool]: + if _is_unsafe_archive_name(member.filename): + return [AuditFinding(location, "archive member path is unsafe")], False, False + if stat.S_ISLNK(member.external_attr >> 16): + return [AuditFinding(location, "archive member is a symbolic link")], False, False + if member.is_dir(): + return [], False, False + is_license = _is_distribution_license(member.filename) + is_metadata = _is_distribution_metadata(member.filename) + if not (is_license or is_metadata) and PurePosixPath(member.filename).suffix.casefold() not in _TEXT_SUFFIXES: + return [], False, False + try: + with archive.open(member) as stream: + content = _read_bounded(stream, expected_size=member.file_size) + except (OSError, ValueError) as error: + return [AuditFinding(location, str(error))], False, False + contains_license = is_license and b"Apache License" in content and b"Version 2.0" in content + declares_license = is_metadata and b"\nLicense-Expression: Apache-2.0\n" in b"\n" + content + return _audit_content(location, member.filename, content), contains_license, declares_license + + +def _audit_tar_member( + archive: tarfile.TarFile, + member: tarfile.TarInfo, + location: str, +) -> list[AuditFinding]: + if _is_unsafe_archive_name(member.name): + return [AuditFinding(location, "archive member path is unsafe")] + if member.issym() or member.islnk(): + return [AuditFinding(location, "archive member is a link")] + if not member.isfile() or PurePosixPath(member.name).suffix.casefold() not in _TEXT_SUFFIXES: + return [] + stream = archive.extractfile(member) + if stream is None: + return [AuditFinding(location, "archive member cannot be read")] + try: + with stream: + content = _read_bounded(stream, expected_size=member.size) + except (OSError, ValueError) as error: + return [AuditFinding(location, str(error))] + return _audit_content(location, member.name, content) + + +def _get_archive_limit_finding(path: Path, *, member_count: int, content_size: int) -> AuditFinding | None: + if member_count > MAXIMUM_ARCHIVE_MEMBERS: + return AuditFinding(_display_path(path), "archive exceeds the member-count scan limit") + if content_size > MAXIMUM_ARCHIVE_CONTENT_SIZE: + return AuditFinding(_display_path(path), "archive exceeds the expanded-content scan limit") + return None + + +def _audit_content(location: str, logical_name: str, content: bytes) -> list[AuditFinding]: + text = content.decode("utf-8", errors="replace") + findings = [ + AuditFinding(location, rule.name) + for rule in _CONTENT_RULES + if rule.pattern.search(logical_name) or rule.pattern.search(text) + ] + if _requires_spdx(logical_name) and not _has_spdx_header(text): + findings.append(AuditFinding(location, "packaged source is missing the NVIDIA Apache-2.0 SPDX header")) + return findings + + +def _read_bounded(stream: BinaryIO, *, expected_size: int) -> bytes: + if expected_size > MAXIMUM_MEMBER_SIZE: + raise ValueError("artifact exceeds the per-file scan limit") + content = stream.read(MAXIMUM_MEMBER_SIZE + 1) + if len(content) > MAXIMUM_MEMBER_SIZE: + raise ValueError("artifact exceeds the per-file scan limit") + return content + + +def _requires_spdx(logical_name: str) -> bool: + normalized = logical_name.replace("\\", "/") + suffix = PurePosixPath(normalized).suffix.casefold() + return suffix in _LICENSED_SOURCE_SUFFIXES and ( + normalized == "entrypoint.sh" + or normalized.startswith("data_designer/slurm/") + or "/src/data_designer/slurm/" in normalized + ) + + +def _has_spdx_header(text: str) -> bool: + header = "\n".join(text.splitlines()[:5]) + return _SPDX_COPYRIGHT in header and _SPDX_LICENSE in header + + +def _is_distribution_license(name: str) -> bool: + normalized = name.casefold() + basename = PurePosixPath(normalized).name + return ( + _SLURM_DIST_INFO_PATTERN.match(normalized) is not None + and basename in {"license", "license.md", "license.txt"} + and (".dist-info/licenses/" in normalized or ".dist-info/" in normalized) + ) + + +def _is_distribution_metadata(name: str) -> bool: + normalized = name.casefold() + return _SLURM_DIST_INFO_PATTERN.match(normalized) is not None and normalized.endswith(".dist-info/metadata") + + +def _is_unsafe_archive_name(name: str) -> bool: + path = PurePosixPath(name) + return path.is_absolute() or ".." in path.parts or "\\" in name + + +def _is_zip_archive(path: Path) -> bool: + return path.suffix.casefold() in {".whl", ".zip"} + + +def _is_tar_archive(path: Path) -> bool: + normalized = path.name.casefold() + return normalized.endswith((".tar", ".tar.gz", ".tgz")) + + +def _display_path(path: Path) -> str: + try: + return path.relative_to(REPOSITORY_ROOT).as_posix() + except ValueError: + return "" + + +def main(arguments: list[str] | None = None) -> int: + """Run the public-artifact audit and print only path-scoped rule names.""" + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("paths", nargs="*", type=Path, help="Files or directories to scan") + parsed = parser.parse_args(arguments) + paths = parsed.paths or [REPOSITORY_ROOT / relative for relative in DEFAULT_ARTIFACTS] + findings = audit_public_artifacts(paths) + if findings: + for finding in findings: + print(f"{finding.location}: {finding.rule}", file=sys.stderr) + return 1 + print(f"Slurm public-artifact audit passed for {len(paths)} target(s).") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/test_slurm_package_install.py b/scripts/test_slurm_package_install.py index f80247355..d28f082a9 100644 --- a/scripts/test_slurm_package_install.py +++ b/scripts/test_slurm_package_install.py @@ -46,6 +46,16 @@ def wheel_metadata(path: Path) -> Message: return BytesParser().parsebytes(wheel.read(metadata_path)) +def audit_public_artifacts(*paths: Path) -> None: + result = run( + [sys.executable, str(REPOSITORY_ROOT / "scripts" / "audit_slurm_public_artifacts.py"), *map(str, paths)], + cwd=REPOSITORY_ROOT, + check=False, + ) + if result.returncode: + raise RuntimeError(result.stdout + result.stderr) + + def build_wheels(uv: str, wheel_directory: Path) -> dict[str, Path]: for package_path in PACKAGE_PATHS: run( @@ -183,6 +193,7 @@ def main() -> None: base_wheel = wheels["data-designer"] leaf_wheel = wheels["data-designer-slurm"] + audit_public_artifacts(leaf_wheel) base_metadata = wheel_metadata(base_wheel) leaf_metadata = wheel_metadata(leaf_wheel) version = base_metadata["Version"] From 104decbb3e17a1bba3b98ca0838934dd5bed07fe Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Thu, 3 Sep 2026 10:53:34 -0600 Subject: [PATCH 2/5] fix Slurm security audit gaps Signed-off-by: Nabin Mulepati --- .../src/data_designer/slurm/security.py | 2 +- .../tests/launcher/test_client.py | 30 ++++++++++++++++++- .../tests/test_public_artifacts.py | 23 ++++++++++++-- scripts/audit_slurm_public_artifacts.py | 15 ++++++++-- 4 files changed, 64 insertions(+), 6 deletions(-) diff --git a/packages/data-designer-slurm/src/data_designer/slurm/security.py b/packages/data-designer-slurm/src/data_designer/slurm/security.py index 4901d7f52..895a191ce 100644 --- a/packages/data-designer-slurm/src/data_designer/slurm/security.py +++ b/packages/data-designer-slurm/src/data_designer/slurm/security.py @@ -11,7 +11,7 @@ _ASSIGNMENT_PATTERN = re.compile( r"(?P(?P[\"']?)(?P-{0,2}[A-Za-z][A-Za-z0-9_.-]*)" - r"(?P=quote)\s*[:=]\s*)(?P\"[^\"]*\"|'[^']*'|[^\s,;]+)" + r"(?P=quote)\s*[:=]\s*)(?P\"[^\"]*\"|'[^']*'|[^\s]+)" ) _OPTION_PATTERN = re.compile(r"(?P(?P--[A-Za-z][A-Za-z0-9.-]*)\s+)(?P\"[^\"]*\"|'[^']*'|[^\s]+)") _AUTHORIZATION_PATTERN = re.compile(r"(?i)(?P\bauthorization\s*[:=]\s*(?:basic|bearer)\s+)(?P[^\s,;]+)") diff --git a/packages/data-designer-slurm/tests/launcher/test_client.py b/packages/data-designer-slurm/tests/launcher/test_client.py index 4246dff0d..9c8335e1d 100644 --- a/packages/data-designer-slurm/tests/launcher/test_client.py +++ b/packages/data-designer-slurm/tests/launcher/test_client.py @@ -193,6 +193,8 @@ def test_client_removes_terminal_controls_from_command_failures(fake_slurm_runne ("diagnostic", "secret"), ( ("HF_TOKEN=super-secret-value", "super-secret-value"), + ("HF_TOKEN=secret;suffix status=failed", "secret;suffix"), + ("HF_TOKEN=secret,suffix status=failed", "secret,suffix"), ('HF_TOKEN="quoted secret value"', "quoted secret value"), ("--api-key plaintext-secret", "plaintext-secret"), ('{"access_token":"json-secret"}', "json-secret"), @@ -200,7 +202,17 @@ def test_client_removes_terminal_controls_from_command_failures(fake_slurm_runne ("https://user:url-secret@example.test/index", "url-secret"), (f"token github_pat_{'a' * 24}", f"github_pat_{'a' * 24}"), ), - ids=("environment", "quoted-environment", "option", "json", "authorization", "url", "known-token"), + ids=( + "environment", + "semicolon-environment", + "comma-environment", + "quoted-environment", + "option", + "json", + "authorization", + "url", + "known-token", + ), ) def test_client_redacts_secrets_from_command_failures( fake_slurm_runner: FakeSlurmRunner, @@ -217,6 +229,22 @@ def test_client_redacts_secrets_from_command_failures( assert "" in str(error.value) +@pytest.mark.parametrize("separator", (";", ","), ids=("semicolon", "comma")) +def test_client_preserves_diagnostic_fields_after_redacted_assignment( + fake_slurm_runner: FakeSlurmRunner, + separator: str, +) -> None: + fake_slurm_runner.script_next( + "squeue", + FakeCommandResponse(stderr=f"HF_TOKEN=secret{separator}suffix status=failed", returncode=2), + ) + + with pytest.raises(SlurmCommandError) as error: + SlurmCommandClient(fake_slurm_runner).query_queue((4101,)) + + assert "HF_TOKEN= status=failed" in str(error.value) + + def test_client_bounds_command_failure_detail(fake_slurm_runner: FakeSlurmRunner) -> None: fake_slurm_runner.script_next("squeue", FakeCommandResponse(stderr="x" * 600, returncode=2)) client = SlurmCommandClient(fake_slurm_runner) diff --git a/packages/data-designer-slurm/tests/test_public_artifacts.py b/packages/data-designer-slurm/tests/test_public_artifacts.py index d0abae0e3..b60db95ee 100644 --- a/packages/data-designer-slurm/tests/test_public_artifacts.py +++ b/packages/data-designer-slurm/tests/test_public_artifacts.py @@ -14,6 +14,7 @@ REPOSITORY_ROOT = Path(__file__).parents[3] AUDIT_SCRIPT = REPOSITORY_ROOT / "scripts" / "audit_slurm_public_artifacts.py" +PACKAGE_LICENSE = REPOSITORY_ROOT / "packages" / "data-designer-slurm" / "LICENSE" SPDX_HEADER = """# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. # SPDX-License-Identifier: Apache-2.0 """ @@ -57,7 +58,7 @@ def test_public_audit_checks_wheel_members_and_license_text(tmp_path: Path) -> N archive.writestr("data_designer/slurm/runtime.py", f"{SPDX_HEADER}\nfrom __future__ import annotations\n") archive.writestr( "data_designer_slurm-1.0.0.dist-info/licenses/LICENSE", - "Apache License\nVersion 2.0\n", + PACKAGE_LICENSE.read_bytes(), ) archive.writestr( "data_designer_slurm-1.0.0.dist-info/METADATA", @@ -69,6 +70,24 @@ def test_public_audit_checks_wheel_members_and_license_text(tmp_path: Path) -> N assert result.returncode == 0, result.stderr +def test_public_audit_rejects_wheel_with_truncated_license(tmp_path: Path) -> None: + wheel = tmp_path / "data_designer_slurm-1.0.0-py3-none-any.whl" + with zipfile.ZipFile(wheel, mode="w") as archive: + archive.writestr( + "data_designer_slurm-1.0.0.dist-info/licenses/LICENSE", + "Apache License\nVersion 2.0\n", + ) + archive.writestr( + "data_designer_slurm-1.0.0.dist-info/METADATA", + "Metadata-Version: 2.5\nLicense-Expression: Apache-2.0\n", + ) + + result = _run_audit(wheel) + + assert result.returncode == 1 + assert "wheel does not contain its declared license text" in result.stderr + + def test_public_audit_rejects_unsafe_or_unlicensed_wheel_members(tmp_path: Path) -> None: wheel = tmp_path / "data_designer_slurm-1.0.0-py3-none-any.whl" with zipfile.ZipFile(wheel, mode="w") as archive: @@ -91,7 +110,7 @@ def test_public_audit_scans_archive_member_names_without_echoing_them(tmp_path: archive.writestr(sensitive_member, SPDX_HEADER) archive.writestr( "data_designer_slurm-1.0.0.dist-info/licenses/LICENSE", - "Apache License\nVersion 2.0\n", + PACKAGE_LICENSE.read_bytes(), ) archive.writestr( "data_designer_slurm-1.0.0.dist-info/METADATA", diff --git a/scripts/audit_slurm_public_artifacts.py b/scripts/audit_slurm_public_artifacts.py index 87b88dfd2..8ce0bcb04 100644 --- a/scripts/audit_slurm_public_artifacts.py +++ b/scripts/audit_slurm_public_artifacts.py @@ -27,6 +27,7 @@ MAXIMUM_ARCHIVE_CONTENT_SIZE = 128 * 1024 * 1024 MAXIMUM_ARCHIVE_MEMBERS = 10_000 REPOSITORY_ROOT = Path(__file__).parents[1] +CANONICAL_PACKAGE_LICENSE = REPOSITORY_ROOT / "packages" / "data-designer-slurm" / "LICENSE" DEFAULT_ARTIFACTS = ( "packages/data-designer-slurm/src", "packages/data-designer-slurm/README.md", @@ -153,6 +154,10 @@ def _audit_zip(path: Path) -> list[AuditFinding]: findings: list[AuditFinding] = [] license_present = False license_declared = False + try: + canonical_license = CANONICAL_PACKAGE_LICENSE.read_bytes() + except OSError: + return [AuditFinding(_display_path(path), "canonical package license cannot be read")] try: with zipfile.ZipFile(path) as archive: members = archive.infolist() @@ -165,7 +170,12 @@ def _audit_zip(path: Path) -> list[AuditFinding]: return [limit_finding] for index, member in enumerate(sorted(members, key=lambda item: item.filename), start=1): location = f"{_display_path(path)}!member-{index}" - member_findings, contains_license, declares_license = _audit_zip_member(archive, member, location) + member_findings, contains_license, declares_license = _audit_zip_member( + archive, + member, + location, + canonical_license, + ) findings.extend(member_findings) license_present = license_present or contains_license license_declared = license_declared or declares_license @@ -203,6 +213,7 @@ def _audit_zip_member( archive: zipfile.ZipFile, member: zipfile.ZipInfo, location: str, + canonical_license: bytes, ) -> tuple[list[AuditFinding], bool, bool]: if _is_unsafe_archive_name(member.filename): return [AuditFinding(location, "archive member path is unsafe")], False, False @@ -219,7 +230,7 @@ def _audit_zip_member( content = _read_bounded(stream, expected_size=member.file_size) except (OSError, ValueError) as error: return [AuditFinding(location, str(error))], False, False - contains_license = is_license and b"Apache License" in content and b"Version 2.0" in content + contains_license = is_license and content == canonical_license declares_license = is_metadata and b"\nLicense-Expression: Apache-2.0\n" in b"\n" + content return _audit_content(location, member.filename, content), contains_license, declares_license From 288025152f39a4f3d4ddde977f3afe468ed186dd Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Thu, 3 Sep 2026 11:12:49 -0600 Subject: [PATCH 3/5] harden adjacent secret redaction Signed-off-by: Nabin Mulepati --- .../src/data_designer/slurm/security.py | 56 +++++++++++++++++-- .../tests/launcher/test_client.py | 46 ++++++++++++--- 2 files changed, 89 insertions(+), 13 deletions(-) diff --git a/packages/data-designer-slurm/src/data_designer/slurm/security.py b/packages/data-designer-slurm/src/data_designer/slurm/security.py index 895a191ce..46d729b07 100644 --- a/packages/data-designer-slurm/src/data_designer/slurm/security.py +++ b/packages/data-designer-slurm/src/data_designer/slurm/security.py @@ -9,9 +9,8 @@ from data_designer.slurm.config.environment import is_secret_bearing_name -_ASSIGNMENT_PATTERN = re.compile( - r"(?P(?P[\"']?)(?P-{0,2}[A-Za-z][A-Za-z0-9_.-]*)" - r"(?P=quote)\s*[:=]\s*)(?P\"[^\"]*\"|'[^']*'|[^\s]+)" +_ASSIGNMENT_START_PATTERN = re.compile( + r"(?P(?P[\"']?)(?P-{0,2}[A-Za-z][A-Za-z0-9_.-]*)(?P=quote)\s*[:=]\s*)" ) _OPTION_PATTERN = re.compile(r"(?P(?P--[A-Za-z][A-Za-z0-9.-]*)\s+)(?P\"[^\"]*\"|'[^']*'|[^\s]+)") _AUTHORIZATION_PATTERN = re.compile(r"(?i)(?P\bauthorization\s*[:=]\s*(?:basic|bearer)\s+)(?P[^\s,;]+)") @@ -30,13 +29,62 @@ def redact_sensitive_text(value: str) -> str: """Redact recognizable credentials without echoing their values.""" redacted = _AUTHORIZATION_PATTERN.sub(lambda match: f"{match.group('prefix')}{_REDACTION}", value) redacted = _URL_USERINFO_PATTERN.sub(lambda match: f"{match.group('scheme')}{_REDACTION}@", redacted) - redacted = _ASSIGNMENT_PATTERN.sub(_redact_named_value, redacted) + redacted = _redact_assignments(redacted) redacted = _OPTION_PATTERN.sub(_redact_named_value, redacted) for pattern in _TOKEN_PATTERNS: redacted = pattern.sub(_REDACTION, redacted) return redacted +def _redact_assignments(value: str) -> str: + """Redact secret-bearing assignments without letting earlier matches overlap them.""" + parts: list[str] = [] + cursor = 0 + while match := _ASSIGNMENT_START_PATTERN.search(value, cursor): + value_end = _find_assignment_value_end(value, match.end()) + parts.append(value[cursor : match.start()]) + if is_secret_bearing_name(match.group("name").lstrip("-")): + parts.append(f"{match.group('prefix')}{_REDACTION}") + else: + parts.append(value[match.start() : value_end]) + cursor = value_end + parts.append(value[cursor:]) + return "".join(parts) + + +def _find_assignment_value_end(value: str, start: int) -> int: + """Find a value boundary while retaining punctuation that belongs to the value.""" + if start < len(value) and value[start] in {'"', "'"}: + return _find_quoted_value_end(value, start) + index = start + while index < len(value): + if value[index].isspace(): + return index + if value[index] in ",;" and _starts_assignment(value, index + 1): + return index + index += 1 + return index + + +def _find_quoted_value_end(value: str, start: int) -> int: + quote = value[start] + index = start + 1 + while index < len(value): + if value[index] == "\\": + index += 2 + elif value[index] == quote: + return index + 1 + else: + index += 1 + return len(value) + + +def _starts_assignment(value: str, start: int) -> bool: + while start < len(value) and value[start].isspace(): + start += 1 + return _ASSIGNMENT_START_PATTERN.match(value, start) is not None + + def _redact_named_value(match: re.Match[str]) -> str: name = match.group("name").lstrip("-") if not is_secret_bearing_name(name): diff --git a/packages/data-designer-slurm/tests/launcher/test_client.py b/packages/data-designer-slurm/tests/launcher/test_client.py index 9c8335e1d..ff6f2e30c 100644 --- a/packages/data-designer-slurm/tests/launcher/test_client.py +++ b/packages/data-designer-slurm/tests/launcher/test_client.py @@ -193,8 +193,6 @@ def test_client_removes_terminal_controls_from_command_failures(fake_slurm_runne ("diagnostic", "secret"), ( ("HF_TOKEN=super-secret-value", "super-secret-value"), - ("HF_TOKEN=secret;suffix status=failed", "secret;suffix"), - ("HF_TOKEN=secret,suffix status=failed", "secret,suffix"), ('HF_TOKEN="quoted secret value"', "quoted secret value"), ("--api-key plaintext-secret", "plaintext-secret"), ('{"access_token":"json-secret"}', "json-secret"), @@ -204,8 +202,6 @@ def test_client_removes_terminal_controls_from_command_failures(fake_slurm_runne ), ids=( "environment", - "semicolon-environment", - "comma-environment", "quoted-environment", "option", "json", @@ -229,20 +225,52 @@ def test_client_redacts_secrets_from_command_failures( assert "" in str(error.value) -@pytest.mark.parametrize("separator", (";", ","), ids=("semicolon", "comma")) -def test_client_preserves_diagnostic_fields_after_redacted_assignment( +@pytest.mark.parametrize( + ("diagnostic", "expected"), + ( + ("HF_TOKEN=secret;status=failed", "HF_TOKEN=;status=failed"), + ("HF_TOKEN=secret,status=failed", "HF_TOKEN=,status=failed"), + ("HF_TOKEN=secret status=failed", "HF_TOKEN= status=failed"), + ("status=failed;HF_TOKEN=secret;job=4", "status=failed;HF_TOKEN=;job=4"), + ("status=failed,HF_TOKEN=secret,job=4", "status=failed,HF_TOKEN=,job=4"), + ("status=failed HF_TOKEN=secret job=4", "status=failed HF_TOKEN= job=4"), + ("status=failed; HF_TOKEN=secret, job=4", "status=failed; HF_TOKEN=, job=4"), + ("status=failed;HF_TOKEN=secret", "status=failed;HF_TOKEN="), + ("status=failed,HF_TOKEN=secret", "status=failed,HF_TOKEN="), + ("status=failed HF_TOKEN=secret", "status=failed HF_TOKEN="), + ("HF_TOKEN=secret;suffix status=failed", "HF_TOKEN= status=failed"), + ("HF_TOKEN=secret,suffix status=failed", "HF_TOKEN= status=failed"), + ), + ids=( + "first-semicolon", + "first-comma", + "first-whitespace", + "middle-semicolon", + "middle-comma", + "middle-whitespace", + "middle-spaced-punctuation", + "last-semicolon", + "last-comma", + "last-whitespace", + "semicolon-inside-value", + "comma-inside-value", + ), +) +def test_client_redacts_adjacent_assignments_without_swallowing_diagnostics( fake_slurm_runner: FakeSlurmRunner, - separator: str, + diagnostic: str, + expected: str, ) -> None: fake_slurm_runner.script_next( "squeue", - FakeCommandResponse(stderr=f"HF_TOKEN=secret{separator}suffix status=failed", returncode=2), + FakeCommandResponse(stderr=diagnostic, returncode=2), ) with pytest.raises(SlurmCommandError) as error: SlurmCommandClient(fake_slurm_runner).query_queue((4101,)) - assert "HF_TOKEN= status=failed" in str(error.value) + assert expected in str(error.value) + assert "HF_TOKEN=secret" not in str(error.value) def test_client_bounds_command_failure_detail(fake_slurm_runner: FakeSlurmRunner) -> None: From 1e0b50c469f713756b34d26130c92838b96fa3f4 Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Thu, 3 Sep 2026 15:08:36 -0600 Subject: [PATCH 4/5] make Slurm redaction fail closed Signed-off-by: Nabin Mulepati --- .../src/data_designer/slurm/security.py | 57 +++++++------------ .../tests/launcher/test_client.py | 46 +++++++++++++-- 2 files changed, 61 insertions(+), 42 deletions(-) diff --git a/packages/data-designer-slurm/src/data_designer/slurm/security.py b/packages/data-designer-slurm/src/data_designer/slurm/security.py index 46d729b07..a01de96c8 100644 --- a/packages/data-designer-slurm/src/data_designer/slurm/security.py +++ b/packages/data-designer-slurm/src/data_designer/slurm/security.py @@ -12,8 +12,8 @@ _ASSIGNMENT_START_PATTERN = re.compile( r"(?P(?P[\"']?)(?P-{0,2}[A-Za-z][A-Za-z0-9_.-]*)(?P=quote)\s*[:=]\s*)" ) -_OPTION_PATTERN = re.compile(r"(?P(?P--[A-Za-z][A-Za-z0-9.-]*)\s+)(?P\"[^\"]*\"|'[^']*'|[^\s]+)") -_AUTHORIZATION_PATTERN = re.compile(r"(?i)(?P\bauthorization\s*[:=]\s*(?:basic|bearer)\s+)(?P[^\s,;]+)") +_OPTION_START_PATTERN = re.compile(r"(?P(?P--[A-Za-z][A-Za-z0-9.-]*)\s+)") +_AUTHORIZATION_PATTERN = re.compile(r"(?i)(?P\bauthorization\s*[:=]\s*(?:basic|bearer)\s+)(?P[^\s]+)") _URL_USERINFO_PATTERN = re.compile(r"(?i)(?P\bhttps?://)[^/@\s]+@") _TOKEN_PATTERNS = ( re.compile(r"\bAKIA[0-9A-Z]{16}\b"), @@ -29,39 +29,37 @@ def redact_sensitive_text(value: str) -> str: """Redact recognizable credentials without echoing their values.""" redacted = _AUTHORIZATION_PATTERN.sub(lambda match: f"{match.group('prefix')}{_REDACTION}", value) redacted = _URL_USERINFO_PATTERN.sub(lambda match: f"{match.group('scheme')}{_REDACTION}@", redacted) - redacted = _redact_assignments(redacted) - redacted = _OPTION_PATTERN.sub(_redact_named_value, redacted) + redacted = _redact_named_values(redacted, _ASSIGNMENT_START_PATTERN) + redacted = _redact_named_values(redacted, _OPTION_START_PATTERN) for pattern in _TOKEN_PATTERNS: redacted = pattern.sub(_REDACTION, redacted) return redacted -def _redact_assignments(value: str) -> str: - """Redact secret-bearing assignments without letting earlier matches overlap them.""" +def _redact_named_values(value: str, start_pattern: re.Pattern[str]) -> str: + """Redact secret-bearing named values without letting earlier matches overlap them.""" parts: list[str] = [] - cursor = 0 - while match := _ASSIGNMENT_START_PATTERN.search(value, cursor): - value_end = _find_assignment_value_end(value, match.end()) - parts.append(value[cursor : match.start()]) - if is_secret_bearing_name(match.group("name").lstrip("-")): - parts.append(f"{match.group('prefix')}{_REDACTION}") - else: - parts.append(value[match.start() : value_end]) - cursor = value_end - parts.append(value[cursor:]) + output_cursor = 0 + search_cursor = 0 + while match := start_pattern.search(value, search_cursor): + if not is_secret_bearing_name(match.group("name").lstrip("-")): + search_cursor = match.end() + continue + value_end = _find_named_value_end(value, match.end()) + parts.append(value[output_cursor : match.start()]) + parts.append(f"{match.group('prefix')}{_REDACTION}") + output_cursor = value_end + search_cursor = value_end + parts.append(value[output_cursor:]) return "".join(parts) -def _find_assignment_value_end(value: str, start: int) -> int: - """Find a value boundary while retaining punctuation that belongs to the value.""" +def _find_named_value_end(value: str, start: int) -> int: + """Find the first unambiguous boundary for a secret-bearing value.""" if start < len(value) and value[start] in {'"', "'"}: return _find_quoted_value_end(value, start) index = start - while index < len(value): - if value[index].isspace(): - return index - if value[index] in ",;" and _starts_assignment(value, index + 1): - return index + while index < len(value) and not value[index].isspace(): index += 1 return index @@ -79,17 +77,4 @@ def _find_quoted_value_end(value: str, start: int) -> int: return len(value) -def _starts_assignment(value: str, start: int) -> bool: - while start < len(value) and value[start].isspace(): - start += 1 - return _ASSIGNMENT_START_PATTERN.match(value, start) is not None - - -def _redact_named_value(match: re.Match[str]) -> str: - name = match.group("name").lstrip("-") - if not is_secret_bearing_name(name): - return match.group(0) - return f"{match.group('prefix')}{_REDACTION}" - - __all__ = ["redact_sensitive_text"] diff --git a/packages/data-designer-slurm/tests/launcher/test_client.py b/packages/data-designer-slurm/tests/launcher/test_client.py index ff6f2e30c..b6512b04b 100644 --- a/packages/data-designer-slurm/tests/launcher/test_client.py +++ b/packages/data-designer-slurm/tests/launcher/test_client.py @@ -195,6 +195,7 @@ def test_client_removes_terminal_controls_from_command_failures(fake_slurm_runne ("HF_TOKEN=super-secret-value", "super-secret-value"), ('HF_TOKEN="quoted secret value"', "quoted secret value"), ("--api-key plaintext-secret", "plaintext-secret"), + ("Authorization: Bearer bearer-secret;suffix status=failed", "bearer-secret;suffix"), ('{"access_token":"json-secret"}', "json-secret"), ("Authorization: Bearer bearer-secret", "bearer-secret"), ("https://user:url-secret@example.test/index", "url-secret"), @@ -204,6 +205,7 @@ def test_client_removes_terminal_controls_from_command_failures(fake_slurm_runne "environment", "quoted-environment", "option", + "authorization-punctuation", "json", "authorization", "url", @@ -228,18 +230,23 @@ def test_client_redacts_secrets_from_command_failures( @pytest.mark.parametrize( ("diagnostic", "expected"), ( - ("HF_TOKEN=secret;status=failed", "HF_TOKEN=;status=failed"), - ("HF_TOKEN=secret,status=failed", "HF_TOKEN=,status=failed"), + ("HF_TOKEN=secret;status=failed job=4", "HF_TOKEN= job=4"), + ("HF_TOKEN=secret,status=failed job=4", "HF_TOKEN= job=4"), ("HF_TOKEN=secret status=failed", "HF_TOKEN= status=failed"), - ("status=failed;HF_TOKEN=secret;job=4", "status=failed;HF_TOKEN=;job=4"), - ("status=failed,HF_TOKEN=secret,job=4", "status=failed,HF_TOKEN=,job=4"), + ("status=failed;HF_TOKEN=secret;job=4 next=ready", "status=failed;HF_TOKEN= next=ready"), + ("status=failed,HF_TOKEN=secret,job=4 next=ready", "status=failed,HF_TOKEN= next=ready"), ("status=failed HF_TOKEN=secret job=4", "status=failed HF_TOKEN= job=4"), - ("status=failed; HF_TOKEN=secret, job=4", "status=failed; HF_TOKEN=, job=4"), + ("status=failed; HF_TOKEN=secret, job=4", "status=failed; HF_TOKEN= job=4"), ("status=failed;HF_TOKEN=secret", "status=failed;HF_TOKEN="), ("status=failed,HF_TOKEN=secret", "status=failed,HF_TOKEN="), ("status=failed HF_TOKEN=secret", "status=failed HF_TOKEN="), + ("status=failed|HF_TOKEN=secret", "status=failed|HF_TOKEN="), + ("status=failed/HF_TOKEN=secret", "status=failed/HF_TOKEN="), ("HF_TOKEN=secret;suffix status=failed", "HF_TOKEN= status=failed"), ("HF_TOKEN=secret,suffix status=failed", "HF_TOKEN= status=failed"), + ("HF_TOKEN=secret;part=value status=failed", "HF_TOKEN= status=failed"), + ("HF_TOKEN=secret,part=value status=failed", "HF_TOKEN= status=failed"), + ('HF_TOKEN="secret";status=failed', "HF_TOKEN=;status=failed"), ), ids=( "first-semicolon", @@ -252,11 +259,16 @@ def test_client_redacts_secrets_from_command_failures( "last-semicolon", "last-comma", "last-whitespace", + "pipe-before-secret", + "slash-before-secret", "semicolon-inside-value", "comma-inside-value", + "assignment-looking-semicolon-suffix", + "assignment-looking-comma-suffix", + "quoted-value-boundary", ), ) -def test_client_redacts_adjacent_assignments_without_swallowing_diagnostics( +def test_client_redacts_adjacent_assignments_fail_closed( fake_slurm_runner: FakeSlurmRunner, diagnostic: str, expected: str, @@ -273,6 +285,28 @@ def test_client_redacts_adjacent_assignments_without_swallowing_diagnostics( assert "HF_TOKEN=secret" not in str(error.value) +@pytest.mark.parametrize( + ("diagnostic", "expected"), + ( + ("--format compact|--api-key secret", "--format compact|--api-key "), + ("--api-key secret;part=value status=failed", "--api-key status=failed"), + ), + ids=("unknown-separator", "assignment-looking-suffix"), +) +def test_client_redacts_option_values_without_overlap( + fake_slurm_runner: FakeSlurmRunner, + diagnostic: str, + expected: str, +) -> None: + fake_slurm_runner.script_next("squeue", FakeCommandResponse(stderr=diagnostic, returncode=2)) + + with pytest.raises(SlurmCommandError) as error: + SlurmCommandClient(fake_slurm_runner).query_queue((4101,)) + + assert expected in str(error.value) + assert "api-key secret" not in str(error.value) + + def test_client_bounds_command_failure_detail(fake_slurm_runner: FakeSlurmRunner) -> None: fake_slurm_runner.script_next("squeue", FakeCommandResponse(stderr="x" * 600, returncode=2)) client = SlurmCommandClient(fake_slurm_runner) From a633ce3a81b4ae77c8ce11932bc717fd2d6281a4 Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Thu, 3 Sep 2026 16:43:57 -0600 Subject: [PATCH 5/5] redact underscore secret options Signed-off-by: Nabin Mulepati --- .../src/data_designer/slurm/security.py | 2 +- .../tests/launcher/test_client.py | 38 ++++++++++++++++--- 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/packages/data-designer-slurm/src/data_designer/slurm/security.py b/packages/data-designer-slurm/src/data_designer/slurm/security.py index a01de96c8..2ba7d0c76 100644 --- a/packages/data-designer-slurm/src/data_designer/slurm/security.py +++ b/packages/data-designer-slurm/src/data_designer/slurm/security.py @@ -12,7 +12,7 @@ _ASSIGNMENT_START_PATTERN = re.compile( r"(?P(?P[\"']?)(?P-{0,2}[A-Za-z][A-Za-z0-9_.-]*)(?P=quote)\s*[:=]\s*)" ) -_OPTION_START_PATTERN = re.compile(r"(?P(?P--[A-Za-z][A-Za-z0-9.-]*)\s+)") +_OPTION_START_PATTERN = re.compile(r"(?P(?P--[A-Za-z][A-Za-z0-9_.-]*)\s+)") _AUTHORIZATION_PATTERN = re.compile(r"(?i)(?P\bauthorization\s*[:=]\s*(?:basic|bearer)\s+)(?P[^\s]+)") _URL_USERINFO_PATTERN = re.compile(r"(?i)(?P\bhttps?://)[^/@\s]+@") _TOKEN_PATTERNS = ( diff --git a/packages/data-designer-slurm/tests/launcher/test_client.py b/packages/data-designer-slurm/tests/launcher/test_client.py index b6512b04b..37a502a8f 100644 --- a/packages/data-designer-slurm/tests/launcher/test_client.py +++ b/packages/data-designer-slurm/tests/launcher/test_client.py @@ -286,25 +286,51 @@ def test_client_redacts_adjacent_assignments_fail_closed( @pytest.mark.parametrize( - ("diagnostic", "expected"), + ("diagnostic", "expected", "secret"), ( - ("--format compact|--api-key secret", "--format compact|--api-key "), - ("--api-key secret;part=value status=failed", "--api-key status=failed"), + ("--format compact|--api-key secret", "--format compact|--api-key ", "secret"), + ("--format compact|--api_key secret", "--format compact|--api_key ", "secret"), + ("--api-key plaintext-secret", "--api-key ", "plaintext-secret"), + ("--api_key plaintext-secret", "--api_key ", "plaintext-secret"), + ('--api_Key "quoted secret value" status=failed', "--api_Key status=failed", "quoted secret value"), + ( + "status=failed/--access_token.key 'single quoted secret' next=ready", + "status=failed/--access_token.key next=ready", + "single quoted secret", + ), + ("--api-key secret;part=value status=failed", "--api-key status=failed", "secret;part=value"), + ("--api_key secret,part=value status=failed", "--api_key status=failed", "secret,part=value"), + ("--access-token_key mixed-secret", "--access-token_key ", "mixed-secret"), + ("--output_format compact status=failed", "--output_format compact status=failed", None), + ), + ids=( + "hyphen-after-unknown-separator", + "underscore-after-unknown-separator", + "hyphen-unquoted", + "underscore-unquoted", + "mixed-case-double-quoted", + "mixed-dot-underscore-single-quoted", + "hyphen-punctuation-suffix", + "underscore-punctuation-suffix", + "mixed-hyphen-underscore", + "nonsecret-underscore-preserved", ), - ids=("unknown-separator", "assignment-looking-suffix"), ) def test_client_redacts_option_values_without_overlap( fake_slurm_runner: FakeSlurmRunner, diagnostic: str, expected: str, + secret: str | None, ) -> None: fake_slurm_runner.script_next("squeue", FakeCommandResponse(stderr=diagnostic, returncode=2)) with pytest.raises(SlurmCommandError) as error: SlurmCommandClient(fake_slurm_runner).query_queue((4101,)) - assert expected in str(error.value) - assert "api-key secret" not in str(error.value) + detail = str(error.value).partition(": ")[2] + assert detail == expected + if secret is not None: + assert secret not in detail def test_client_bounds_command_failure_detail(fake_slurm_runner: FakeSlurmRunner) -> None: