From 1db14edf13fa4df4c3c23db1877cc797eed7209e Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Thu, 3 Sep 2026 10:27:52 -0600 Subject: [PATCH 01/12] 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/security.py | 47 +++ .../tests/launcher/test_client.py | 28 ++ .../tests/runtime/test_bundle.py | 1 + .../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 + 11 files changed, 848 insertions(+), 1 deletion(-) 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 cc8c33046..f691c26dd 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 58e203105..0059ab7a0 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 @@ -31,6 +31,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, SchedulerJobIdentity _IDENTIFIER_PATTERN = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$") @@ -344,7 +345,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/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 d17e5eb25..c3e406a7d 100644 --- a/packages/data-designer-slurm/tests/launcher/test_client.py +++ b/packages/data-designer-slurm/tests/launcher/test_client.py @@ -317,6 +317,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 5b73ad071..abfc4a885 100644 --- a/packages/data-designer-slurm/tests/runtime/test_bundle.py +++ b/packages/data-designer-slurm/tests/runtime/test_bundle.py @@ -48,6 +48,7 @@ def test_runtime_bundle_is_deterministic_content_addressed_and_restrictive(tmp_p entrypoint = archive.extractfile("entrypoint.sh") assert entrypoint is not None entrypoint_content = entrypoint.read() + assert b"SPDX-License-Identifier: Apache-2.0" in entrypoint_content assert b"python3 -m data_designer.slurm.runtime.entrypoint" not in entrypoint_content assert b"--retry-plan-sha256" in entrypoint_content assert b"--effective-resume-mode" in entrypoint_content 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 c2c696633..412a1b00b 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( @@ -199,6 +209,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 329ceb7c94e547c2c1addaa65b2f865c77b47644 Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Thu, 3 Sep 2026 10:53:34 -0600 Subject: [PATCH 02/12] 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 c3e406a7d..dcc86fb6b 100644 --- a/packages/data-designer-slurm/tests/launcher/test_client.py +++ b/packages/data-designer-slurm/tests/launcher/test_client.py @@ -321,6 +321,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"), @@ -328,7 +330,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, @@ -345,6 +357,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 054f8498943769d3727bd0fad6c253d671ad895e Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Thu, 3 Sep 2026 11:12:49 -0600 Subject: [PATCH 03/12] 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 dcc86fb6b..728f20a8e 100644 --- a/packages/data-designer-slurm/tests/launcher/test_client.py +++ b/packages/data-designer-slurm/tests/launcher/test_client.py @@ -321,8 +321,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"), @@ -332,8 +330,6 @@ def test_client_removes_terminal_controls_from_command_failures(fake_slurm_runne ), ids=( "environment", - "semicolon-environment", - "comma-environment", "quoted-environment", "option", "json", @@ -357,20 +353,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 75a5db2f734c78fd848f29c6ba86ae1aca1814d9 Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Thu, 3 Sep 2026 15:08:36 -0600 Subject: [PATCH 04/12] 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 728f20a8e..3fff8eb99 100644 --- a/packages/data-designer-slurm/tests/launcher/test_client.py +++ b/packages/data-designer-slurm/tests/launcher/test_client.py @@ -323,6 +323,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"), @@ -332,6 +333,7 @@ def test_client_removes_terminal_controls_from_command_failures(fake_slurm_runne "environment", "quoted-environment", "option", + "authorization-punctuation", "json", "authorization", "url", @@ -356,18 +358,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", @@ -380,11 +387,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, @@ -401,6 +413,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 dcb59fccd4afebd0712cfb1239cad78059f642a9 Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Thu, 3 Sep 2026 16:43:57 -0600 Subject: [PATCH 05/12] 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 3fff8eb99..3fe821c9e 100644 --- a/packages/data-designer-slurm/tests/launcher/test_client.py +++ b/packages/data-designer-slurm/tests/launcher/test_client.py @@ -414,25 +414,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: From 1043ec08fe05210bbc9308eecde942f4d30d8b6c Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Tue, 8 Sep 2026 11:31:38 -0600 Subject: [PATCH 06/12] harden Slurm security audit boundaries Signed-off-by: Nabin Mulepati --- .../data_designer/slurm/launcher/client.py | 6 +- .../src/data_designer/slurm/security.py | 4 +- .../tests/launcher/test_client.py | 56 ++++++ .../tests/test_public_artifacts.py | 100 +++++++++- scripts/audit_slurm_public_artifacts.py | 179 +++++++++++------- 5 files changed, 269 insertions(+), 76 deletions(-) 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 0059ab7a0..2254ba46e 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 @@ -343,10 +343,10 @@ 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) + redacted = redact_sensitive_text(value) + sanitized = "".join(" " if unicodedata.category(character).startswith("C") else character for character in redacted) normalized = " ".join(sanitized.split()) - redacted = redact_sensitive_text(normalized) - return redacted if len(redacted) <= limit else f"{redacted[: limit - 3]}..." + return normalized if len(normalized) <= limit else f"{normalized[: limit - 3]}..." def _format_error_detail(error: BaseException) -> str: 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 2ba7d0c76..86d7b1bfd 100644 --- a/packages/data-designer-slurm/src/data_designer/slurm/security.py +++ b/packages/data-designer-slurm/src/data_designer/slurm/security.py @@ -13,8 +13,8 @@ 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+)") -_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]+@") +_AUTHORIZATION_PATTERN = re.compile(r"(?i)(?P\bauthorization\s*[:=]\s*)[^\r\n]*") +_URL_USERINFO_PATTERN = re.compile(r"(?i)(?P\b[A-Za-z][A-Za-z0-9+.-]*://)[^/\s?#]*@") _TOKEN_PATTERNS = ( re.compile(r"\bAKIA[0-9A-Z]{16}\b"), re.compile(r"\bgithub_pat_[A-Za-z0-9_]{20,}\b"), diff --git a/packages/data-designer-slurm/tests/launcher/test_client.py b/packages/data-designer-slurm/tests/launcher/test_client.py index 3fe821c9e..8a0fe1163 100644 --- a/packages/data-designer-slurm/tests/launcher/test_client.py +++ b/packages/data-designer-slurm/tests/launcher/test_client.py @@ -355,6 +355,62 @@ def test_client_redacts_secrets_from_command_failures( assert "" in str(error.value) +@pytest.mark.parametrize( + ("diagnostic", "expected", "secret"), + ( + ("Authorization: Token token-secret status=failed", "Authorization: ", "token-secret"), + ("Authorization=ApiKey key-secret", "Authorization=", "key-secret"), + ("authorization: opaque secret value", "authorization: ", "opaque secret value"), + ( + "Authorization: Custom custom-secret\nstatus=failed", + "Authorization: status=failed", + "custom-secret", + ), + ), + ids=("token", "api-key", "bare", "newline-boundary"), +) +def test_client_redacts_complete_authorization_values( + fake_slurm_runner: FakeSlurmRunner, + diagnostic: str, + expected: str, + secret: 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,)) + + detail = str(error.value).partition(": ")[2] + assert detail == expected + assert secret not in detail + + +@pytest.mark.parametrize( + ("diagnostic", "expected"), + ( + ( + "https://user:first@second@example.test/index", + "https://@example.test/index", + ), + ("ssh://user:secret@example.test/repository", "ssh://@example.test/repository"), + ("git+ssh://user:p@ss@example.test/repository", "git+ssh://@example.test/repository"), + ("https://example.test/path@owner", "https://example.test/path@owner"), + ), + ids=("last-at", "ssh", "compound-scheme", "at-after-path"), +) +def test_client_redacts_general_uri_userinfo( + 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 str(error.value).partition(": ")[2] == expected + + @pytest.mark.parametrize( ("diagnostic", "expected"), ( diff --git a/packages/data-designer-slurm/tests/test_public_artifacts.py b/packages/data-designer-slurm/tests/test_public_artifacts.py index b60db95ee..7edc982bf 100644 --- a/packages/data-designer-slurm/tests/test_public_artifacts.py +++ b/packages/data-designer-slurm/tests/test_public_artifacts.py @@ -85,7 +85,57 @@ def test_public_audit_rejects_wheel_with_truncated_license(tmp_path: Path) -> No result = _run_audit(wheel) assert result.returncode == 1 - assert "wheel does not contain its declared license text" in result.stderr + assert "wheel does not contain exactly one canonical license text" in result.stderr + + +def test_public_audit_rejects_multiple_wheel_distribution_roots(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", + PACKAGE_LICENSE.read_bytes(), + ) + archive.writestr( + "decoy-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 must contain exactly one .dist-info root" in result.stderr + + +@pytest.mark.parametrize( + ("extra_member", "expected"), + ( + ( + "data_designer_slurm-1.0.0.dist-info/licenses/LICENSE.txt", + "wheel does not contain exactly one canonical license text", + ), + ( + "data_designer_slurm-1.0.0.dist-info/metadata", + "wheel does not contain exactly one Apache-2.0 metadata record", + ), + ), + ids=("license", "metadata"), +) +def test_public_audit_rejects_duplicate_wheel_distribution_records( + tmp_path: Path, + extra_member: str, + expected: str, +) -> None: + wheel = tmp_path / "data_designer_slurm-1.0.0-py3-none-any.whl" + metadata = "Metadata-Version: 2.5\nLicense-Expression: Apache-2.0\n" + with zipfile.ZipFile(wheel, mode="w") as archive: + archive.writestr("data_designer_slurm-1.0.0.dist-info/licenses/LICENSE", PACKAGE_LICENSE.read_bytes()) + archive.writestr("data_designer_slurm-1.0.0.dist-info/METADATA", metadata) + archive.writestr(extra_member, metadata if extra_member.endswith("metadata") else PACKAGE_LICENSE.read_bytes()) + + result = _run_audit(wheel) + + assert result.returncode == 1 + assert expected in result.stderr def test_public_audit_rejects_unsafe_or_unlicensed_wheel_members(tmp_path: Path) -> None: @@ -99,8 +149,7 @@ def test_public_audit_rejects_unsafe_or_unlicensed_wheel_members(tmp_path: Path) 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 + assert "wheel must contain exactly one .dist-info root" in result.stderr def test_public_audit_scans_archive_member_names_without_echoing_them(tmp_path: Path) -> None: @@ -124,6 +173,17 @@ def test_public_audit_scans_archive_member_names_without_echoing_them(tmp_path: assert sensitive_member not in result.stderr +def test_public_audit_scans_unknown_suffix_archive_members(tmp_path: Path) -> None: + archive_path = tmp_path / "artifacts.zip" + with zipfile.ZipFile(archive_path, mode="w") as archive: + archive.writestr("credentials.env", f"token=github_pat_{'a' * 24}\n") + + result = _run_audit(archive_path) + + assert result.returncode == 1 + assert "GitHub token" 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}" @@ -154,6 +214,40 @@ def test_public_audit_rejects_explicit_symbolic_link_without_disclosing_its_pare assert str(tmp_path) not in result.stderr +def test_public_audit_rejects_symbolic_link_named_as_generated_cache(tmp_path: Path) -> None: + artifacts = tmp_path / "artifacts" + artifacts.mkdir() + target = tmp_path / "target" + target.mkdir() + (artifacts / "__pycache__").symlink_to(target, target_is_directory=True) + + result = _run_audit(artifacts) + + assert result.returncode == 1 + assert "symbolic-link artifact requires explicit review" in result.stderr + + +@pytest.mark.parametrize("filename", ("credentials.env", "records.csv", "METADATA", "LICENSE")) +def test_public_audit_scans_content_with_unknown_or_empty_suffixes(tmp_path: Path, filename: str) -> None: + artifact = tmp_path / filename + artifact.write_text(f"token=github_pat_{'a' * 24}\n") + + result = _run_audit(artifact) + + assert result.returncode == 1 + assert "GitHub token" in result.stderr + + +def test_public_audit_does_not_allow_test_sentinels_outside_exact_sources(tmp_path: Path) -> None: + artifact = tmp_path / "test_credentials.py" + artifact.write_text('secret = "super-secret-token"\n') + + result = _run_audit(artifact) + + assert result.returncode == 1 + assert "plaintext secret assignment" 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)], diff --git a/scripts/audit_slurm_public_artifacts.py b/scripts/audit_slurm_public_artifacts.py index 8ce0bcb04..6aa93fdf4 100644 --- a/scripts/audit_slurm_public_artifacts.py +++ b/scripts/audit_slurm_public_artifacts.py @@ -3,11 +3,10 @@ """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. +The default scope covers deployable source, test source and fixtures, package +metadata, and release scripts. A small path-scoped allowlist masks only exact +synthetic test sentinels. Generic example.test hosts, loopback addresses, and +/workspace paths are the only implicit test-data allowances. """ from __future__ import annotations @@ -33,39 +32,35 @@ "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", + "packages/data-designer-slurm/tests", "plans/850/data-designer-contract.md", "plans/870/slurm-early-security-review.md", + "scripts/publish.sh", "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/") +_SLURM_DIST_INFO_PATTERN = re.compile(r"^data_designer_slurm-[^/]+\.dist-info$") +_TEST_SOURCE_SENTINELS = { + "packages/data-designer-slurm/tests/config/test_loading_builder.py": ( + '"clusters": {secret: profile_catalog.clusters["primary"]}', + "super-secret-token", + "sk_live_ABC123XYZ", + ), + "packages/data-designer-slurm/tests/contracts/test_config_records.py": ("plaintext-secret",), + "packages/data-designer-slurm/tests/launcher/test_client.py": ( + "Authorization: Bearer bearer-secret;suffix status=failed", + "Authorization: Bearer bearer-secret", + ), + "packages/data-designer-slurm/tests/planning/test_compiler.py": ("super-secret-token",), + "packages/data-designer-slurm/tests/test_public_artifacts.py": ( + "/home/specific-user/run", + "10.23.45.67", + "service.internal.nvidia.com", + "super-secret-token", + ), +} @dataclass(frozen=True, slots=True) @@ -84,6 +79,16 @@ class AuditFinding: rule: str +@dataclass(frozen=True, slots=True) +class _ZipMemberAudit: + findings: tuple[AuditFinding, ...] + dist_info_root: str | None = None + is_license: bool = False + license_matches: bool = False + is_metadata: bool = False + metadata_declares_license: bool = False + + _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")), @@ -125,6 +130,8 @@ def audit_public_artifacts(paths: Iterable[Path]) -> tuple[AuditFinding, ...]: findings.append( AuditFinding(_display_path(child), "symbolic-link artifact requires explicit review") ) + elif "__pycache__" in child.parts: + continue elif child.is_file(): findings.extend(_audit_file(child)) else: @@ -137,8 +144,6 @@ def _audit_file(path: Path) -> list[AuditFinding]: 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) @@ -147,13 +152,17 @@ def _audit_file(path: Path) -> list[AuditFinding]: except ValueError as error: return [AuditFinding(_display_path(path), str(error))] location = _display_path(path) - return _audit_content(location, location, content) + return _audit_content( + location, + location, + content, + allowed_sentinels=_TEST_SOURCE_SENTINELS.get(location, ()), + ) def _audit_zip(path: Path) -> list[AuditFinding]: findings: list[AuditFinding] = [] - license_present = False - license_declared = False + member_audits: list[_ZipMemberAudit] = [] try: canonical_license = CANONICAL_PACKAGE_LICENSE.read_bytes() except OSError: @@ -170,21 +179,18 @@ 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( + member_audit = _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 + findings.extend(member_audit.findings) + member_audits.append(member_audit) 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")) + if path.suffix.casefold() == ".whl": + findings.extend(_audit_wheel_distribution(path, member_audits)) return findings @@ -214,25 +220,48 @@ def _audit_zip_member( member: zipfile.ZipInfo, location: str, canonical_license: bytes, -) -> tuple[list[AuditFinding], bool, bool]: +) -> _ZipMemberAudit: + dist_info_root = _get_dist_info_root(member.filename) if _is_unsafe_archive_name(member.filename): - return [AuditFinding(location, "archive member path is unsafe")], False, False + return _ZipMemberAudit((AuditFinding(location, "archive member path is unsafe"),)) if stat.S_ISLNK(member.external_attr >> 16): - return [AuditFinding(location, "archive member is a symbolic link")], False, False + return _ZipMemberAudit((AuditFinding(location, "archive member is a symbolic link"),)) 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 + return _ZipMemberAudit((), dist_info_root=dist_info_root) + is_license = _is_distribution_license(member.filename, dist_info_root) + is_metadata = _is_distribution_metadata(member.filename, dist_info_root) 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 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 + return _ZipMemberAudit((AuditFinding(location, str(error)),), dist_info_root=dist_info_root) + return _ZipMemberAudit( + findings=tuple(_audit_content(location, member.filename, content)), + dist_info_root=dist_info_root, + is_license=is_license, + license_matches=is_license and content == canonical_license, + is_metadata=is_metadata, + metadata_declares_license=is_metadata and b"\nLicense-Expression: Apache-2.0\n" in b"\n" + content, + ) + + +def _audit_wheel_distribution(path: Path, members: Iterable[_ZipMemberAudit]) -> list[AuditFinding]: + member_audits = tuple(members) + roots = frozenset(member.dist_info_root for member in member_audits if member.dist_info_root is not None) + location = _display_path(path) + if len(roots) != 1: + return [AuditFinding(location, "wheel must contain exactly one .dist-info root")] + root = next(iter(roots)) + if _SLURM_DIST_INFO_PATTERN.fullmatch(root.casefold()) is None: + return [AuditFinding(location, "wheel .dist-info root does not identify data-designer-slurm")] + licenses = tuple(member for member in member_audits if member.dist_info_root == root and member.is_license) + metadata = tuple(member for member in member_audits if member.dist_info_root == root and member.is_metadata) + findings: list[AuditFinding] = [] + if len(licenses) != 1 or not licenses[0].license_matches: + findings.append(AuditFinding(location, "wheel does not contain exactly one canonical license text")) + if len(metadata) != 1 or not metadata[0].metadata_declares_license: + findings.append(AuditFinding(location, "wheel does not contain exactly one Apache-2.0 metadata record")) + return findings def _audit_tar_member( @@ -244,7 +273,7 @@ def _audit_tar_member( 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: + if not member.isfile(): return [] stream = archive.extractfile(member) if stream is None: @@ -265,8 +294,16 @@ def _get_archive_limit_finding(path: Path, *, member_count: int, content_size: i return None -def _audit_content(location: str, logical_name: str, content: bytes) -> list[AuditFinding]: +def _audit_content( + location: str, + logical_name: str, + content: bytes, + *, + allowed_sentinels: Iterable[str] = (), +) -> list[AuditFinding]: text = content.decode("utf-8", errors="replace") + for sentinel in sorted(allowed_sentinels, key=len, reverse=True): + text = text.replace(sentinel, "") findings = [ AuditFinding(location, rule.name) for rule in _CONTENT_RULES @@ -301,19 +338,25 @@ def _has_spdx_header(text: str) -> bool: 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 _get_dist_info_root(name: str) -> str | None: + parts = PurePosixPath(name).parts + if not parts or not parts[0].casefold().endswith(".dist-info"): + return None + return parts[0] + + +def _is_distribution_license(name: str, root: str | None) -> bool: + if root is None: + return False + parts = tuple(part.casefold() for part in PurePosixPath(name).parts) + license_names = {"license", "license.md", "license.txt"} + return (len(parts) == 2 and parts[1] in license_names) or ( + len(parts) == 3 and parts[1] == "licenses" and parts[2] in license_names ) -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_distribution_metadata(name: str, root: str | None) -> bool: + return root is not None and name.casefold() == f"{root.casefold()}/metadata" def _is_unsafe_archive_name(name: str) -> bool: From 9ce72164dde43ef60b467c4f68eedc45198b0fad Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Tue, 8 Sep 2026 11:40:44 -0600 Subject: [PATCH 07/12] fix control-obscured Slurm secrets Signed-off-by: Nabin Mulepati --- .../data_designer/slurm/launcher/client.py | 14 +++++++-- .../tests/launcher/test_client.py | 30 +++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) 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 2254ba46e..dc8a309f0 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 @@ -342,10 +342,18 @@ def _validate_argument(value: str, *, field_name: str) -> None: raise ValueError(f"{field_name} must not contain control characters") +def _normalize_control_boundaries(value: str) -> str: + """Expose control-obscured token boundaries while retaining line boundaries.""" + return "".join( + character if character in "\r\n" or not unicodedata.category(character).startswith("C") else " " + for character in value + ) + + def _normalize_bounded_text(value: str, *, limit: int = 512) -> str: - redacted = redact_sensitive_text(value) - sanitized = "".join(" " if unicodedata.category(character).startswith("C") else character for character in redacted) - normalized = " ".join(sanitized.split()) + sanitized = _normalize_control_boundaries(value) + redacted = redact_sensitive_text(sanitized) + normalized = " ".join(redacted.split()) return normalized if len(normalized) <= limit else f"{normalized[: limit - 3]}..." diff --git a/packages/data-designer-slurm/tests/launcher/test_client.py b/packages/data-designer-slurm/tests/launcher/test_client.py index 8a0fe1163..7cccb9aad 100644 --- a/packages/data-designer-slurm/tests/launcher/test_client.py +++ b/packages/data-designer-slurm/tests/launcher/test_client.py @@ -317,6 +317,36 @@ def test_client_removes_terminal_controls_from_command_failures(fake_slurm_runne assert "\x1b" not in str(error.value) +@pytest.mark.parametrize( + ("diagnostic", "expected", "secret"), + ( + ("--api-key\x00plaintext-secret", "--api-key ", "plaintext-secret"), + ("--api_key\x1funderscore-secret", "--api_key ", "underscore-secret"), + ("--access-token\u200bformat-secret", "--access-token ", "format-secret"), + ( + "Authorization: Custom control-secret\x00value\nstatus=failed", + "Authorization: status=failed", + "control-secret", + ), + ), + ids=("nul", "unit-separator", "format-control", "authorization-line-boundary"), +) +def test_client_redacts_secrets_obscured_by_control_characters( + fake_slurm_runner: FakeSlurmRunner, + diagnostic: str, + expected: str, + secret: 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,)) + + detail = str(error.value).partition(": ")[2] + assert detail == expected + assert secret not in detail + + @pytest.mark.parametrize( ("diagnostic", "secret"), ( From f138e88e3c2d976744dec52ddcd6f375e29f2158 Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Tue, 8 Sep 2026 11:51:27 -0600 Subject: [PATCH 08/12] fix control-split secret redaction Signed-off-by: Nabin Mulepati --- .../data_designer/slurm/launcher/client.py | 14 +--- .../src/data_designer/slurm/security.py | 74 +++++++++++++++++-- .../tests/launcher/test_client.py | 29 +++++++- 3 files changed, 96 insertions(+), 21 deletions(-) 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 dc8a309f0..cc40fbeff 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 @@ -8,7 +8,6 @@ import os import re import subprocess -import unicodedata from collections.abc import Mapping, Sequence from dataclasses import dataclass from datetime import datetime, timedelta @@ -31,7 +30,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.security import redact_sensitive_diagnostic from data_designer.slurm.state import SchedulerIdentity, SchedulerJobIdentity _IDENTIFIER_PATTERN = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$") @@ -342,17 +341,8 @@ def _validate_argument(value: str, *, field_name: str) -> None: raise ValueError(f"{field_name} must not contain control characters") -def _normalize_control_boundaries(value: str) -> str: - """Expose control-obscured token boundaries while retaining line boundaries.""" - return "".join( - character if character in "\r\n" or not unicodedata.category(character).startswith("C") else " " - for character in value - ) - - def _normalize_bounded_text(value: str, *, limit: int = 512) -> str: - sanitized = _normalize_control_boundaries(value) - redacted = redact_sensitive_text(sanitized) + redacted = redact_sensitive_diagnostic(value) normalized = " ".join(redacted.split()) return normalized if len(normalized) <= limit else f"{normalized[: limit - 3]}..." 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 86d7b1bfd..ef6e4e036 100644 --- a/packages/data-designer-slurm/src/data_designer/slurm/security.py +++ b/packages/data-designer-slurm/src/data_designer/slurm/security.py @@ -6,6 +6,7 @@ from __future__ import annotations import re +import unicodedata from data_designer.slurm.config.environment import is_secret_bearing_name @@ -27,16 +28,54 @@ 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_named_values(redacted, _ASSIGNMENT_START_PATTERN) - redacted = _redact_named_values(redacted, _OPTION_START_PATTERN) + return _redact_sensitive_text(value, replacement=_REDACTION) + + +def redact_sensitive_diagnostic(value: str) -> str: + """Redact secrets both inside and separated by control characters.""" + placeholder = _select_redaction_placeholder(value) + redacted = _redact_sensitive_text(value, replacement=placeholder) + normalized_boundaries = _normalize_control_boundaries(redacted) + protected = _redact_sensitive_text( + normalized_boundaries, + replacement=placeholder, + protected_replacement=placeholder, + ) + return protected.replace(placeholder, _REDACTION) + + +def _redact_sensitive_text( + value: str, + *, + replacement: str, + protected_replacement: str | None = None, +) -> str: + redacted = _AUTHORIZATION_PATTERN.sub(lambda match: f"{match.group('prefix')}{replacement}", value) + redacted = _URL_USERINFO_PATTERN.sub(lambda match: f"{match.group('scheme')}{replacement}@", redacted) + redacted = _redact_named_values( + redacted, + _ASSIGNMENT_START_PATTERN, + replacement=replacement, + protected_replacement=protected_replacement, + ) + redacted = _redact_named_values( + redacted, + _OPTION_START_PATTERN, + replacement=replacement, + protected_replacement=protected_replacement, + ) for pattern in _TOKEN_PATTERNS: - redacted = pattern.sub(_REDACTION, redacted) + redacted = pattern.sub(replacement, redacted) return redacted -def _redact_named_values(value: str, start_pattern: re.Pattern[str]) -> str: +def _redact_named_values( + value: str, + start_pattern: re.Pattern[str], + *, + replacement: str, + protected_replacement: str | None, +) -> str: """Redact secret-bearing named values without letting earlier matches overlap them.""" parts: list[str] = [] output_cursor = 0 @@ -45,9 +84,12 @@ def _redact_named_values(value: str, start_pattern: re.Pattern[str]) -> str: if not is_secret_bearing_name(match.group("name").lstrip("-")): search_cursor = match.end() continue + if protected_replacement is not None and value.startswith(protected_replacement, match.end()): + search_cursor = match.end() + len(protected_replacement) + continue value_end = _find_named_value_end(value, match.end()) parts.append(value[output_cursor : match.start()]) - parts.append(f"{match.group('prefix')}{_REDACTION}") + parts.append(f"{match.group('prefix')}{replacement}") output_cursor = value_end search_cursor = value_end parts.append(value[output_cursor:]) @@ -77,4 +119,20 @@ def _find_quoted_value_end(value: str, start: int) -> int: return len(value) -__all__ = ["redact_sensitive_text"] +def _normalize_control_boundaries(value: str) -> str: + """Expose control-obscured token boundaries while retaining line boundaries.""" + return "".join( + character if character in "\r\n" or not unicodedata.category(character).startswith("C") else " " + for character in value + ) + + +def _select_redaction_placeholder(value: str) -> str: + """Return a printable redaction marker guaranteed absent from the input.""" + placeholder = "" + while placeholder in value: + placeholder = f"<{placeholder}>" + return placeholder + + +__all__ = ["redact_sensitive_diagnostic", "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 7cccb9aad..4e049bb5c 100644 --- a/packages/data-designer-slurm/tests/launcher/test_client.py +++ b/packages/data-designer-slurm/tests/launcher/test_client.py @@ -328,8 +328,35 @@ def test_client_removes_terminal_controls_from_command_failures(fake_slurm_runne "Authorization: status=failed", "control-secret", ), + ("HF_TOKEN=secret\x00suffix status=failed", "HF_TOKEN= status=failed", "suffix"), + ( + "https://user:secret\x00suffix@example.test/path status=failed", + "https://@example.test/path status=failed", + "suffix", + ), + ("HF_TOKEN\x00=boundary-secret", "HF_TOKEN =", "boundary-secret"), + ( + 'HF_TOKEN="first-secret";status=failed --api-key\x00second-secret next=ready', + "HF_TOKEN=;status=failed --api-key next=ready", + "second-secret", + ), + ( + "HF_TOKEN=\x00suffix status=failed", + "HF_TOKEN= status=failed", + "suffix", + ), + ), + ids=( + "nul", + "unit-separator", + "format-control", + "authorization-line-boundary", + "assignment-secret-suffix", + "uri-userinfo-secret-suffix", + "assignment-hidden-boundary", + "raw-and-normalized-pass", + "placeholder-collision", ), - ids=("nul", "unit-separator", "format-control", "authorization-line-boundary"), ) def test_client_redacts_secrets_obscured_by_control_characters( fake_slurm_runner: FakeSlurmRunner, From 131c9ee7a450d629d45153490d67ff428399a2ba Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Tue, 8 Sep 2026 12:15:17 -0600 Subject: [PATCH 09/12] fix whitespace-control secret suffixes Signed-off-by: Nabin Mulepati --- .../src/data_designer/slurm/security.py | 26 +++++----- .../tests/launcher/test_client.py | 48 +++++++++++++++++++ 2 files changed, 62 insertions(+), 12 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 ef6e4e036..1e93654ed 100644 --- a/packages/data-designer-slurm/src/data_designer/slurm/security.py +++ b/packages/data-designer-slurm/src/data_designer/slurm/security.py @@ -33,15 +33,17 @@ def redact_sensitive_text(value: str) -> str: def redact_sensitive_diagnostic(value: str) -> str: """Redact secrets both inside and separated by control characters.""" - placeholder = _select_redaction_placeholder(value) - redacted = _redact_sensitive_text(value, replacement=placeholder) - normalized_boundaries = _normalize_control_boundaries(redacted) + control_placeholder = _select_placeholder(value, name="control") + redaction_placeholder = _select_placeholder(value, name="redaction") + scan_view = _replace_control_characters(value, replacement=control_placeholder) + redacted = _redact_sensitive_text(scan_view, replacement=redaction_placeholder) + normalized_boundaries = redacted.replace(control_placeholder, " ") protected = _redact_sensitive_text( normalized_boundaries, - replacement=placeholder, - protected_replacement=placeholder, + replacement=redaction_placeholder, + protected_replacement=redaction_placeholder, ) - return protected.replace(placeholder, _REDACTION) + return protected.replace(redaction_placeholder, _REDACTION) def _redact_sensitive_text( @@ -119,17 +121,17 @@ def _find_quoted_value_end(value: str, start: int) -> int: return len(value) -def _normalize_control_boundaries(value: str) -> str: - """Expose control-obscured token boundaries while retaining line boundaries.""" +def _replace_control_characters(value: str, *, replacement: str) -> str: + """Replace non-line controls while retaining diagnostic line boundaries.""" return "".join( - character if character in "\r\n" or not unicodedata.category(character).startswith("C") else " " + character if character in "\r\n" or not unicodedata.category(character).startswith("C") else replacement for character in value ) -def _select_redaction_placeholder(value: str) -> str: - """Return a printable redaction marker guaranteed absent from the input.""" - placeholder = "" +def _select_placeholder(value: str, *, name: str) -> str: + """Return a named printable marker guaranteed absent from the input.""" + placeholder = f"" while placeholder in value: placeholder = f"<{placeholder}>" return placeholder diff --git a/packages/data-designer-slurm/tests/launcher/test_client.py b/packages/data-designer-slurm/tests/launcher/test_client.py index 4e049bb5c..71f6653fd 100644 --- a/packages/data-designer-slurm/tests/launcher/test_client.py +++ b/packages/data-designer-slurm/tests/launcher/test_client.py @@ -374,6 +374,54 @@ def test_client_redacts_secrets_obscured_by_control_characters( assert secret not in detail +@pytest.mark.parametrize( + "control", + ( + pytest.param("\t", id="tab"), + pytest.param("\v", id="vertical-tab"), + pytest.param("\f", id="form-feed"), + pytest.param("\x1c", id="file-separator"), + pytest.param("\x1d", id="group-separator"), + pytest.param("\x1e", id="record-separator"), + pytest.param("\x1f", id="unit-separator"), + pytest.param("\x85", id="next-line"), + ), +) +def test_client_redacts_whitespace_control_credential_suffixes( + fake_slurm_runner: FakeSlurmRunner, + control: str, +) -> None: + diagnostic = ( + f"HF_TOKEN=assignment-secret{control}suffix status=failed " + f"https://user:uri-secret{control}suffix@example.test/path" + ) + fake_slurm_runner.script_next("squeue", FakeCommandResponse(stderr=diagnostic, returncode=2)) + + with pytest.raises(SlurmCommandError) as error: + SlurmCommandClient(fake_slurm_runner).query_queue((4101,)) + + detail = str(error.value).partition(": ")[2] + assert detail == "HF_TOKEN= status=failed https://@example.test/path" + assert "secret" not in detail + assert "suffix" not in detail + + +@pytest.mark.parametrize("line_break", ("\n", "\r", "\r\n"), ids=("line-feed", "carriage-return", "crlf")) +def test_client_retains_unambiguous_diagnostic_line_boundaries( + fake_slurm_runner: FakeSlurmRunner, + line_break: str, +) -> None: + fake_slurm_runner.script_next( + "squeue", + FakeCommandResponse(stderr=f"HF_TOKEN=secret{line_break}status=failed", returncode=2), + ) + + with pytest.raises(SlurmCommandError) as error: + SlurmCommandClient(fake_slurm_runner).query_queue((4101,)) + + assert str(error.value).endswith("HF_TOKEN= status=failed") + + @pytest.mark.parametrize( ("diagnostic", "secret"), ( From ae64c9c0d997c6c9cd3f1e3d9e084113d691e9bd Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Thu, 10 Sep 2026 07:09:17 -0600 Subject: [PATCH 10/12] harden archive content classification Detect explicitly supplied ZIP and tar inputs from their content when filenames use opaque suffixes. Reject unsupported tar special members while continuing to allow ordinary directories. Signed-off-by: Nabin Mulepati --- .../tests/test_public_artifacts.py | 55 +++++++++++++++++++ scripts/audit_slurm_public_artifacts.py | 18 +++++- 2 files changed, 70 insertions(+), 3 deletions(-) diff --git a/packages/data-designer-slurm/tests/test_public_artifacts.py b/packages/data-designer-slurm/tests/test_public_artifacts.py index 7edc982bf..31c05a550 100644 --- a/packages/data-designer-slurm/tests/test_public_artifacts.py +++ b/packages/data-designer-slurm/tests/test_public_artifacts.py @@ -184,6 +184,31 @@ def test_public_audit_scans_unknown_suffix_archive_members(tmp_path: Path) -> No assert "GitHub token" in result.stderr +def test_public_audit_detects_zip_archive_by_content(tmp_path: Path) -> None: + archive_path = tmp_path / "opaque-artifact" + with zipfile.ZipFile(archive_path, mode="w") as archive: + archive.writestr("credentials.env", f"token=github_pat_{'a' * 24}\n") + + result = _run_audit(archive_path) + + assert result.returncode == 1 + assert "GitHub token" in result.stderr + + +def test_public_audit_detects_tar_archive_by_content(tmp_path: Path) -> None: + archive_path = tmp_path / "opaque-artifact" + secret = f"nvapi-{'a' * 24}".encode() + with tarfile.open(archive_path, mode="w:gz") as archive: + member = tarfile.TarInfo("credentials.env") + member.size = len(secret) + archive.addfile(member, BytesIO(secret)) + + result = _run_audit(archive_path) + + assert result.returncode == 1 + assert "NGC API key" 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}" @@ -201,6 +226,36 @@ def test_public_audit_checks_runtime_tar_content_and_entrypoint_license(tmp_path assert secret not in result.stderr +@pytest.mark.parametrize( + "member_type", + (tarfile.FIFOTYPE, tarfile.CHRTYPE, tarfile.BLKTYPE), + ids=("fifo", "character-device", "block-device"), +) +def test_public_audit_rejects_tar_special_members(tmp_path: Path, member_type: bytes) -> None: + archive_path = tmp_path / "runtime.tar" + with tarfile.open(archive_path, mode="w") as archive: + member = tarfile.TarInfo("unsupported-member") + member.type = member_type + archive.addfile(member) + + result = _run_audit(archive_path) + + assert result.returncode == 1 + assert "archive member type is unsupported" in result.stderr + + +def test_public_audit_allows_tar_directories(tmp_path: Path) -> None: + archive_path = tmp_path / "runtime.tar" + with tarfile.open(archive_path, mode="w") as archive: + directory = tarfile.TarInfo("runtime") + directory.type = tarfile.DIRTYPE + archive.addfile(directory) + + result = _run_audit(archive_path) + + assert result.returncode == 0, 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") diff --git a/scripts/audit_slurm_public_artifacts.py b/scripts/audit_slurm_public_artifacts.py index 6aa93fdf4..5cefe8a3e 100644 --- a/scripts/audit_slurm_public_artifacts.py +++ b/scripts/audit_slurm_public_artifacts.py @@ -273,8 +273,10 @@ def _audit_tar_member( 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(): + if member.isdir(): return [] + if not member.isfile(): + return [AuditFinding(location, "archive member type is unsupported")] stream = archive.extractfile(member) if stream is None: return [AuditFinding(location, "archive member cannot be read")] @@ -365,12 +367,22 @@ def _is_unsafe_archive_name(name: str) -> bool: def _is_zip_archive(path: Path) -> bool: - return path.suffix.casefold() in {".whl", ".zip"} + if path.suffix.casefold() in {".whl", ".zip"}: + return True + try: + return zipfile.is_zipfile(path) + except OSError: + return False def _is_tar_archive(path: Path) -> bool: normalized = path.name.casefold() - return normalized.endswith((".tar", ".tar.gz", ".tgz")) + if normalized.endswith((".tar", ".tar.gz", ".tgz")): + return True + try: + return tarfile.is_tarfile(path) + except OSError: + return False def _display_path(path: Path) -> str: From 9dfde938c4ccd6f45cef1fc6eb0c0e2c55f3ab72 Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Thu, 10 Sep 2026 09:49:34 -0600 Subject: [PATCH 11/12] close Slurm security boundary gaps Handle serialized authorization headers and Unicode whitespace without exposing credential suffixes. Keep public-artifact scanning fail-closed for non-regular files and unreadable ZIP members, and narrow test sentinels to complete values. Signed-off-by: Nabin Mulepati --- .../src/data_designer/slurm/security.py | 31 +++++++-- .../tests/launcher/test_client.py | 37 ++++++++++- .../tests/test_public_artifacts.py | 65 ++++++++++++++++++- plans/870/slurm-early-security-review.md | 4 +- scripts/audit_slurm_public_artifacts.py | 32 +++++++-- 5 files changed, 156 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 1e93654ed..6816915fd 100644 --- a/packages/data-designer-slurm/src/data_designer/slurm/security.py +++ b/packages/data-designer-slurm/src/data_designer/slurm/security.py @@ -14,7 +14,10 @@ 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+)") -_AUTHORIZATION_PATTERN = re.compile(r"(?i)(?P\bauthorization\s*[:=]\s*)[^\r\n]*") +_AUTHORIZATION_PATTERN = re.compile( + r"(?i)(?P(?P[\"']?)\bauthorization(?P=key_quote)\s*[:=]\s*)" + r"(?P\"(?:\\.|[^\"\\])*\"|'(?:\\.|[^'\\])*'|[^\r\n]*)" +) _URL_USERINFO_PATTERN = re.compile(r"(?i)(?P\b[A-Za-z][A-Za-z0-9+.-]*://)[^/\s?#]*@") _TOKEN_PATTERNS = ( re.compile(r"\bAKIA[0-9A-Z]{16}\b"), @@ -52,7 +55,14 @@ def _redact_sensitive_text( replacement: str, protected_replacement: str | None = None, ) -> str: - redacted = _AUTHORIZATION_PATTERN.sub(lambda match: f"{match.group('prefix')}{replacement}", value) + redacted = _AUTHORIZATION_PATTERN.sub( + lambda match: _redact_authorization_value( + match, + replacement=replacement, + protected_replacement=protected_replacement, + ), + value, + ) redacted = _URL_USERINFO_PATTERN.sub(lambda match: f"{match.group('scheme')}{replacement}@", redacted) redacted = _redact_named_values( redacted, @@ -71,6 +81,17 @@ def _redact_sensitive_text( return redacted +def _redact_authorization_value( + match: re.Match[str], + *, + replacement: str, + protected_replacement: str | None, +) -> str: + if protected_replacement is not None and match.group("value").startswith(protected_replacement): + return match.group(0) + return f"{match.group('prefix')}{replacement}" + + def _redact_named_values( value: str, start_pattern: re.Pattern[str], @@ -122,9 +143,11 @@ def _find_quoted_value_end(value: str, start: int) -> int: def _replace_control_characters(value: str, *, replacement: str) -> str: - """Replace non-line controls while retaining diagnostic line boundaries.""" + """Replace ambiguous non-line boundaries before scanning for secrets.""" return "".join( - character if character in "\r\n" or not unicodedata.category(character).startswith("C") else replacement + character + if character in "\r\n " or not (character.isspace() or unicodedata.category(character).startswith("C")) + else replacement for character in value ) diff --git a/packages/data-designer-slurm/tests/launcher/test_client.py b/packages/data-designer-slurm/tests/launcher/test_client.py index 71f6653fd..715ad7fe5 100644 --- a/packages/data-designer-slurm/tests/launcher/test_client.py +++ b/packages/data-designer-slurm/tests/launcher/test_client.py @@ -406,6 +406,36 @@ def test_client_redacts_whitespace_control_credential_suffixes( assert "suffix" not in detail +@pytest.mark.parametrize( + "boundary", + ( + pytest.param("\u00a0", id="no-break-space"), + pytest.param("\u1680", id="ogham-space"), + pytest.param("\u2007", id="figure-space"), + pytest.param("\u202f", id="narrow-no-break-space"), + pytest.param("\u205f", id="medium-mathematical-space"), + pytest.param("\u3000", id="ideographic-space"), + ), +) +def test_client_redacts_unicode_whitespace_credential_suffixes( + fake_slurm_runner: FakeSlurmRunner, + boundary: str, +) -> None: + diagnostic = ( + f"HF_TOKEN=assignment-secret{boundary}suffix status=failed " + f"https://user:uri-secret{boundary}suffix@example.test/path" + ) + fake_slurm_runner.script_next("squeue", FakeCommandResponse(stderr=diagnostic, returncode=2)) + + with pytest.raises(SlurmCommandError) as error: + SlurmCommandClient(fake_slurm_runner).query_queue((4101,)) + + detail = str(error.value).partition(": ")[2] + assert detail == "HF_TOKEN= status=failed https://@example.test/path" + assert "secret" not in detail + assert "suffix" not in detail + + @pytest.mark.parametrize("line_break", ("\n", "\r", "\r\n"), ids=("line-feed", "carriage-return", "crlf")) def test_client_retains_unambiguous_diagnostic_line_boundaries( fake_slurm_runner: FakeSlurmRunner, @@ -471,8 +501,13 @@ def test_client_redacts_secrets_from_command_failures( "Authorization: status=failed", "custom-secret", ), + ( + '{"Authorization": "Bearer serialized-secret-value", "status": "failed"}', + '{"Authorization": , "status": "failed"}', + "serialized-secret-value", + ), ), - ids=("token", "api-key", "bare", "newline-boundary"), + ids=("token", "api-key", "bare", "newline-boundary", "serialized-header"), ) def test_client_redacts_complete_authorization_values( fake_slurm_runner: FakeSlurmRunner, diff --git a/packages/data-designer-slurm/tests/test_public_artifacts.py b/packages/data-designer-slurm/tests/test_public_artifacts.py index 31c05a550..73b9f1361 100644 --- a/packages/data-designer-slurm/tests/test_public_artifacts.py +++ b/packages/data-designer-slurm/tests/test_public_artifacts.py @@ -3,6 +3,7 @@ from __future__ import annotations +import os import subprocess import sys import tarfile @@ -52,6 +53,18 @@ def test_public_audit_reports_rule_without_echoing_sensitive_content( assert str(tmp_path) not in result.stderr +def test_public_audit_detects_quoted_authorization_header(tmp_path: Path) -> None: + artifact = tmp_path / "runtime.log" + serialized_header = '{"Author' + 'ization": "Bearer ' + "serialized-secret-value" + '"}\n' + artifact.write_text(serialized_header) + + result = _run_audit(artifact) + + assert result.returncode == 1 + assert "authorization credential" in result.stderr + assert "serialized-secret-value" 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: @@ -173,6 +186,25 @@ def test_public_audit_scans_archive_member_names_without_echoing_them(tmp_path: assert sensitive_member not in result.stderr +def test_public_audit_sanitizes_encrypted_zip_member_failures(tmp_path: Path) -> None: + archive_path = tmp_path / "artifacts.zip" + sensitive_member = "sensitive-member-name.txt" + with zipfile.ZipFile(archive_path, mode="w") as archive: + archive.writestr(sensitive_member, "safe\n") + content = bytearray(archive_path.read_bytes()) + local_header = content.index(b"PK\x03\x04") + central_header = content.index(b"PK\x01\x02") + content[local_header + 6] |= 1 + content[central_header + 8] |= 1 + archive_path.write_bytes(content) + + result = _run_audit(archive_path) + + assert result.returncode == 1 + assert "archive member cannot be read" in result.stderr + assert sensitive_member not in result.stderr + + def test_public_audit_scans_unknown_suffix_archive_members(tmp_path: Path) -> None: archive_path = tmp_path / "artifacts.zip" with zipfile.ZipFile(archive_path, mode="w") as archive: @@ -282,6 +314,19 @@ def test_public_audit_rejects_symbolic_link_named_as_generated_cache(tmp_path: P assert "symbolic-link artifact requires explicit review" in result.stderr +@pytest.mark.parametrize("scan_directory", (False, True), ids=("explicit", "directory")) +def test_public_audit_rejects_fifo_without_opening_it(tmp_path: Path, scan_directory: bool) -> None: + artifacts = tmp_path / "artifacts" + artifacts.mkdir() + fifo = artifacts / "named-pipe" + os.mkfifo(fifo) + + result = _run_audit(artifacts if scan_directory else fifo) + + assert result.returncode == 1 + assert "artifact is not a regular file" in result.stderr + + @pytest.mark.parametrize("filename", ("credentials.env", "records.csv", "METADATA", "LICENSE")) def test_public_audit_scans_content_with_unknown_or_empty_suffixes(tmp_path: Path, filename: str) -> None: artifact = tmp_path / filename @@ -303,11 +348,27 @@ def test_public_audit_does_not_allow_test_sentinels_outside_exact_sources(tmp_pa assert "plaintext secret assignment" in result.stderr -def _run_audit(*paths: Path) -> subprocess.CompletedProcess[str]: +def test_public_audit_does_not_mask_sentinel_prefixes(tmp_path: Path) -> None: + audit_script = tmp_path / "scripts" / "audit_slurm_public_artifacts.py" + audit_script.parent.mkdir() + audit_script.write_bytes(AUDIT_SCRIPT.read_bytes()) + artifact = tmp_path / "packages" / "data-designer-slurm" / "tests" / "config" / "test_loading_builder.py" + artifact.parent.mkdir(parents=True) + extended_sentinel = "super-secret-token" + "-production-value" + artifact.write_text(f'secret = "{extended_sentinel}"\n') + + result = _run_audit(artifact, audit_script=audit_script) + + assert result.returncode == 1 + assert "plaintext secret assignment" in result.stderr + + +def _run_audit(*paths: Path, audit_script: Path = AUDIT_SCRIPT) -> subprocess.CompletedProcess[str]: return subprocess.run( - [sys.executable, str(AUDIT_SCRIPT), *(str(path) for path in paths)], + [sys.executable, str(audit_script), *(str(path) for path in paths)], cwd=REPOSITORY_ROOT, check=False, capture_output=True, text=True, + timeout=10, ) diff --git a/plans/870/slurm-early-security-review.md b/plans/870/slurm-early-security-review.md index 02b455b8d..c135f1b38 100644 --- a/plans/870/slurm-early-security-review.md +++ b/plans/870/slurm-early-security-review.md @@ -38,8 +38,8 @@ dependent implementation and documentation must still be frozen and validated to 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. + `example.test` hosts, loopback addresses, and `/workspace` paths. Its default scope includes the complete Python test + tree and masks only exact, path-scoped synthetic sentinel values before applying the same high-confidence rules. ## Commands for this review slice diff --git a/scripts/audit_slurm_public_artifacts.py b/scripts/audit_slurm_public_artifacts.py index 5cefe8a3e..236f8608b 100644 --- a/scripts/audit_slurm_public_artifacts.py +++ b/scripts/audit_slurm_public_artifacts.py @@ -42,21 +42,25 @@ _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$") +_SENTINEL_VALUE_CHARACTERS = r"A-Za-z0-9+/_.:@-" _TEST_SOURCE_SENTINELS = { "packages/data-designer-slurm/tests/config/test_loading_builder.py": ( '"clusters": {secret: profile_catalog.clusters["primary"]}', "super-secret-token", + "super-secret-token: x", "sk_live_ABC123XYZ", ), "packages/data-designer-slurm/tests/contracts/test_config_records.py": ("plaintext-secret",), "packages/data-designer-slurm/tests/launcher/test_client.py": ( "Authorization: Bearer bearer-secret;suffix status=failed", "Authorization: Bearer bearer-secret", + '{"Authorization": "Bearer serialized-secret-value", "status": "failed"}', ), "packages/data-designer-slurm/tests/planning/test_compiler.py": ("super-secret-token",), "packages/data-designer-slurm/tests/test_public_artifacts.py": ( "/home/specific-user/run", "10.23.45.67", + "data_designer/slurm/10.23.45.67.py", "service.internal.nvidia.com", "super-secret-token", ), @@ -102,7 +106,10 @@ class _ZipMemberAudit: 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( + "authorization credential", + re.compile(r"(?i)(?P[\"']?)\bauthorization(?P=quote)\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"), @@ -132,7 +139,9 @@ def audit_public_artifacts(paths: Iterable[Path]) -> tuple[AuditFinding, ...]: ) elif "__pycache__" in child.parts: continue - elif child.is_file(): + elif child.is_dir(): + continue + else: findings.extend(_audit_file(child)) else: findings.extend(_audit_file(path)) @@ -140,6 +149,12 @@ def audit_public_artifacts(paths: Iterable[Path]) -> tuple[AuditFinding, ...]: def _audit_file(path: Path) -> list[AuditFinding]: + try: + mode = path.lstat().st_mode + except OSError: + return [AuditFinding(_display_path(path), "artifact cannot be inspected")] + if not stat.S_ISREG(mode): + return [AuditFinding(_display_path(path), "artifact is not a regular file")] if _is_zip_archive(path): return _audit_zip(path) if _is_tar_archive(path): @@ -233,8 +248,13 @@ def _audit_zip_member( try: with archive.open(member) as stream: content = _read_bounded(stream, expected_size=member.file_size) - except (OSError, ValueError) as error: + except ValueError as error: return _ZipMemberAudit((AuditFinding(location, str(error)),), dist_info_root=dist_info_root) + except (OSError, RuntimeError, NotImplementedError, zipfile.BadZipFile): + return _ZipMemberAudit( + (AuditFinding(location, "archive member cannot be read"),), + dist_info_root=dist_info_root, + ) return _ZipMemberAudit( findings=tuple(_audit_content(location, member.filename, content)), dist_info_root=dist_info_root, @@ -305,7 +325,11 @@ def _audit_content( ) -> list[AuditFinding]: text = content.decode("utf-8", errors="replace") for sentinel in sorted(allowed_sentinels, key=len, reverse=True): - text = text.replace(sentinel, "") + text = re.sub( + rf"(?", + text, + ) findings = [ AuditFinding(location, rule.name) for rule in _CONTENT_RULES From 91d156a910adf4a9c087d834fb8e8294c586b71e Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Thu, 10 Sep 2026 09:59:02 -0600 Subject: [PATCH 12/12] redact complete Authorization diagnostics Treat an Authorization field as credential-bearing through its line boundary even when its value is quoted. This keeps control-obscured suffixes from surviving the protected redaction pass. Signed-off-by: Nabin Mulepati --- .../src/data_designer/slurm/security.py | 22 ++----------------- .../tests/launcher/test_client.py | 18 ++++++++++++++- 2 files changed, 19 insertions(+), 21 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 6816915fd..ed9a97ec2 100644 --- a/packages/data-designer-slurm/src/data_designer/slurm/security.py +++ b/packages/data-designer-slurm/src/data_designer/slurm/security.py @@ -16,7 +16,7 @@ _OPTION_START_PATTERN = re.compile(r"(?P(?P--[A-Za-z][A-Za-z0-9_.-]*)\s+)") _AUTHORIZATION_PATTERN = re.compile( r"(?i)(?P(?P[\"']?)\bauthorization(?P=key_quote)\s*[:=]\s*)" - r"(?P\"(?:\\.|[^\"\\])*\"|'(?:\\.|[^'\\])*'|[^\r\n]*)" + r"[^\r\n]*" ) _URL_USERINFO_PATTERN = re.compile(r"(?i)(?P\b[A-Za-z][A-Za-z0-9+.-]*://)[^/\s?#]*@") _TOKEN_PATTERNS = ( @@ -55,14 +55,7 @@ def _redact_sensitive_text( replacement: str, protected_replacement: str | None = None, ) -> str: - redacted = _AUTHORIZATION_PATTERN.sub( - lambda match: _redact_authorization_value( - match, - replacement=replacement, - protected_replacement=protected_replacement, - ), - value, - ) + redacted = _AUTHORIZATION_PATTERN.sub(lambda match: f"{match.group('prefix')}{replacement}", value) redacted = _URL_USERINFO_PATTERN.sub(lambda match: f"{match.group('scheme')}{replacement}@", redacted) redacted = _redact_named_values( redacted, @@ -81,17 +74,6 @@ def _redact_sensitive_text( return redacted -def _redact_authorization_value( - match: re.Match[str], - *, - replacement: str, - protected_replacement: str | None, -) -> str: - if protected_replacement is not None and match.group("value").startswith(protected_replacement): - return match.group(0) - return f"{match.group('prefix')}{replacement}" - - def _redact_named_values( value: str, start_pattern: re.Pattern[str], diff --git a/packages/data-designer-slurm/tests/launcher/test_client.py b/packages/data-designer-slurm/tests/launcher/test_client.py index 715ad7fe5..e4be4ea4a 100644 --- a/packages/data-designer-slurm/tests/launcher/test_client.py +++ b/packages/data-designer-slurm/tests/launcher/test_client.py @@ -503,7 +503,7 @@ def test_client_redacts_secrets_from_command_failures( ), ( '{"Authorization": "Bearer serialized-secret-value", "status": "failed"}', - '{"Authorization": , "status": "failed"}', + '{"Authorization": ', "serialized-secret-value", ), ), @@ -525,6 +525,22 @@ def test_client_redacts_complete_authorization_values( assert secret not in detail +def test_client_redacts_quoted_authorization_value_with_control_obscured_suffix( + fake_slurm_runner: FakeSlurmRunner, +) -> None: + scheme = "Bear" + "er" + diagnostic = f'Authorization: "{scheme} token"\x00secret-suffix\nstatus=failed' + fake_slurm_runner.script_next("squeue", FakeCommandResponse(stderr=diagnostic, returncode=2)) + + with pytest.raises(SlurmCommandError) as error: + SlurmCommandClient(fake_slurm_runner).query_queue((4101,)) + + detail = str(error.value).partition(": ")[2] + assert detail == "Authorization: status=failed" + assert "token" not in detail + assert "suffix" not in detail + + @pytest.mark.parametrize( ("diagnostic", "expected"), (