From cb7c2f9073715811ef1811c6a5d1152d07e4575e Mon Sep 17 00:00:00 2001 From: Skylar Payne <4830598+skylarbpayne@users.noreply.github.com> Date: Wed, 15 Jul 2026 10:48:17 -0700 Subject: [PATCH 1/3] INT-PKG-META: package owned dashboard payload manifest --- pyproject.toml | 8 + src/hermes_workflows/package_resources.py | 86 ++++++++++- src/hermes_workflows/plugin_install.py | 22 ++- .../plugin_payload_manifest.v1.json | 2 +- tests/test_package_resource_contract.py | 45 ++++-- tests/test_package_resources.py | 140 ++++++++++++++++++ tests/test_plugin_install.py | 22 +++ 7 files changed, 306 insertions(+), 19 deletions(-) create mode 100644 tests/test_package_resources.py diff --git a/pyproject.toml b/pyproject.toml index 759da8b..4578d86 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,6 +41,14 @@ packages = ["src/hermes_workflows"] [tool.hatch.build.targets.wheel.force-include] "src/hermes_workflows/plugin_payload_manifest.v1.json" = "hermes_workflows/plugin_payload_manifest.v1.json" +"src/hermes_workflows/plugin_payload/hermes-workflows-approvals/dashboard/dist/index.js" = "hermes_workflows/plugin_payload/hermes-workflows-approvals/dashboard/dist/index.js" +"src/hermes_workflows/plugin_payload/hermes-workflows-approvals/dashboard/dist/style.css" = "hermes_workflows/plugin_payload/hermes-workflows-approvals/dashboard/dist/style.css" + +[tool.hatch.build.targets.sdist] +artifacts = [ + "src/hermes_workflows/plugin_payload/hermes-workflows-approvals/dashboard/dist/index.js", + "src/hermes_workflows/plugin_payload/hermes-workflows-approvals/dashboard/dist/style.css", +] [dependency-groups] dev = ["pytest>=7", "build>=1", "tomli>=2; python_version<'3.11'"] diff --git a/src/hermes_workflows/package_resources.py b/src/hermes_workflows/package_resources.py index fdc2404..a349f3e 100644 --- a/src/hermes_workflows/package_resources.py +++ b/src/hermes_workflows/package_resources.py @@ -3,6 +3,8 @@ import hashlib import json import re +import shutil +import stat from dataclasses import dataclass from importlib import metadata, resources from pathlib import Path, PurePosixPath @@ -12,6 +14,14 @@ _SCHEMA_VERSION = 1 _DISTRIBUTION_NAME = "hermes-workflows" _MANIFEST_RESOURCE = "plugin_payload_manifest.v1.json" +_CANONICAL_PAYLOAD_PATHS = ( + "plugin_payload/hermes-workflows-approvals/__init__.py", + "plugin_payload/hermes-workflows-approvals/dashboard/dist/index.js", + "plugin_payload/hermes-workflows-approvals/dashboard/dist/style.css", + "plugin_payload/hermes-workflows-approvals/dashboard/manifest.json", + "plugin_payload/hermes-workflows-approvals/dashboard/plugin_api.py", + "plugin_payload/hermes-workflows-approvals/plugin.yaml", +) _IDENTIFIER_PATTERN = re.compile(r"^[a-z0-9][a-z0-9_.-]{0,127}$") _SHA256_PATTERN = re.compile(r"^[0-9a-f]{64}$") _FILE_FIELDS = frozenset({"schema_version", "path", "sha256", "size_bytes"}) @@ -185,15 +195,38 @@ def manifest_from_json(value: str) -> PackageResourceManifestV1: return manifest +def _resource_bytes(relative: str) -> bytes: + resource = resources.files("hermes_workflows") + for part in relative.split("/"): + resource = resource.joinpath(part) + return resource.read_bytes() + + +def _validated_payload_bytes(manifest: PackageResourceManifestV1) -> Tuple[Tuple[PackageResourceFileV1, bytes], ...]: + validated = [] + for entry in manifest.files: + try: + data = _resource_bytes(entry.path) + except (FileNotFoundError, IsADirectoryError, OSError) as exc: + raise ValueError(f"packaged payload resource is missing or unreadable: {entry.path}") from exc + if len(data) != entry.size_bytes or hashlib.sha256(data).hexdigest() != entry.sha256: + raise ValueError(f"packaged payload resource does not match its manifest: {entry.path}") + validated.append((entry, data)) + return tuple(validated) + + def foundation_manifest() -> PackageResourceManifestV1: text = resources.files("hermes_workflows").joinpath(_MANIFEST_RESOURCE).read_text(encoding="utf-8") manifest = manifest_from_json(text) if manifest.owner_id != _DISTRIBUTION_NAME or manifest.package_name != _DISTRIBUTION_NAME: raise ValueError("foundation manifest owner and package must be hermes-workflows") - if manifest.payload_root != "plugin_payload" or manifest.files: - raise ValueError("foundation manifest must declare the empty plugin_payload foundation") + if manifest.payload_root != "plugin_payload": + raise ValueError("foundation manifest payload_root must equal plugin_payload") + if tuple(entry.path for entry in manifest.files) != _CANONICAL_PAYLOAD_PATHS: + raise ValueError("foundation manifest must declare the exact canonical plugin payload") if text != canonical_manifest_json(manifest): raise ValueError("foundation manifest resource must contain canonical JSON") + _validated_payload_bytes(manifest) return manifest @@ -202,14 +235,51 @@ def ownership_key(manifest: PackageResourceManifestV1) -> str: return hashlib.sha256(canonical.encode("utf-8")).hexdigest() +def _validate_new_destination(root: Path) -> None: + if not root.is_absolute(): + raise ValueError("package payload destination must be absolute") + if ".." in root.parts: + raise ValueError("package payload destination must not contain traversal components") + + current = Path(root.anchor) + for component in root.parts[1:-1]: + current = current / component + try: + mode = current.lstat().st_mode + except FileNotFoundError: + raise ValueError(f"package payload destination parent does not exist: {current}") + if stat.S_ISLNK(mode): + raise ValueError(f"package payload destination contains symlink ancestor: {current}") + if not stat.S_ISDIR(mode): + raise ValueError(f"package payload destination ancestor is not a directory: {current}") + + def write_package_payload( manifest: PackageResourceManifestV1, destination: PathLike, ) -> Tuple[Path, ...]: - """Return without touching the filesystem for the empty foundation payload.""" + """Copy the exact validated package payload into a new destination root.""" - if not isinstance(manifest, PackageResourceManifestV1): - raise TypeError("manifest must be a PackageResourceManifestV1") - if manifest.files: - raise NotImplementedError("package payload copying is outside the foundation seam") - return () + canonical = canonical_manifest_json(manifest) + packaged = foundation_manifest() + if canonical != canonical_manifest_json(packaged): + raise ValueError("manifest must exactly match the packaged payload manifest") + payload = _validated_payload_bytes(packaged) + root = Path(destination) + _validate_new_destination(root) + if root.exists() or root.is_symlink(): + raise FileExistsError(f"refusing to replace existing destination: {root}") + + root.mkdir() + written = [] + try: + for entry, data in payload: + path = root / entry.path + path.parent.mkdir(parents=True, exist_ok=True) + with path.open("xb") as handle: + handle.write(data) + written.append(path) + except Exception: + shutil.rmtree(root) + raise + return tuple(written) diff --git a/src/hermes_workflows/plugin_install.py b/src/hermes_workflows/plugin_install.py index 154944a..e0c6a13 100644 --- a/src/hermes_workflows/plugin_install.py +++ b/src/hermes_workflows/plugin_install.py @@ -12,6 +12,7 @@ from pathlib import Path, PurePosixPath from typing import Any, Dict, Mapping, Optional, Sequence, Tuple, Union +from . import package_resources from .package_resources import installed_package_version @@ -203,6 +204,11 @@ def _tree_files(root: Path, *, error_type: type[PluginInstallError]) -> Tuple[st return tuple(sorted(discovered)) +def _is_installer_generated_bytecode(relative: str) -> bool: + parts = PurePosixPath(relative).parts + return "__pycache__" in parts and relative.endswith(".pyc") + + def _plugin_yaml_identity(path: Path) -> Tuple[str, str]: _regular_file(path, error_type=PayloadValidationError, label="plugin.yaml") name = None @@ -252,10 +258,13 @@ def inspect_payload( *, expected_package_version: Optional[str] = None, ) -> PayloadDescriptor: + packaged_payload = payload_root is None root = canonical_payload_root() if payload_root is None else Path(payload_root) if not root.is_absolute(): root = root.resolve() actual_files = _tree_files(root, error_type=PayloadValidationError) + if packaged_payload: + actual_files = tuple(relative for relative in actual_files if not _is_installer_generated_bytecode(relative)) if actual_files != PAYLOAD_FILES: missing = sorted(set(PAYLOAD_FILES) - set(actual_files)) unexpected = sorted(set(actual_files) - set(PAYLOAD_FILES)) @@ -274,6 +283,17 @@ def inspect_payload( path = root / relative _regular_file(path, error_type=PayloadValidationError, label=relative) files.append({"path": relative, "sha256": _sha256_file(path), "size_bytes": path.stat().st_size}) + if packaged_payload: + try: + manifest = package_resources.foundation_manifest() + except (TypeError, ValueError, OSError) as exc: + raise PayloadValidationError(f"package payload manifest validation failed: {exc}") from exc + manifest_paths = tuple(f"plugin_payload/{PLUGIN_NAME}/{relative}" for relative in PAYLOAD_FILES) + if tuple(entry.path for entry in manifest.files) != manifest_paths: + raise PayloadValidationError("package payload manifest paths do not match the canonical payload") + for entry, actual in zip(manifest.files, files): + if entry.sha256 != actual["sha256"] or entry.size_bytes != actual["size_bytes"]: + raise PayloadValidationError(f"package payload manifest bytes do not match: {entry.path}") unsigned = { "schema_version": RECEIPT_SCHEMA_VERSION, "owner_id": OWNER_ID, @@ -658,7 +678,7 @@ def _install_or_upgrade( ) -> PluginLifecycleReport: validated_home = _validated_profile_home(profile_home) source = canonical_payload_root() if payload_root is None else Path(payload_root).resolve() - descriptor = inspect_payload(source, expected_package_version=expected_package_version) + descriptor = inspect_payload(payload_root, expected_package_version=expected_package_version) home, plugins, destination, rollback = _profile_paths(validated_home) _recover_interrupted(plugins, destination, rollback) if require_existing and not destination.exists(): diff --git a/src/hermes_workflows/plugin_payload_manifest.v1.json b/src/hermes_workflows/plugin_payload_manifest.v1.json index 0e67581..03a6cb7 100644 --- a/src/hermes_workflows/plugin_payload_manifest.v1.json +++ b/src/hermes_workflows/plugin_payload_manifest.v1.json @@ -1 +1 @@ -{"files":[],"owner_id":"hermes-workflows","package_name":"hermes-workflows","package_version":"0.0.1rc1","payload_root":"plugin_payload","schema_version":1} \ No newline at end of file +{"files":[{"path":"plugin_payload/hermes-workflows-approvals/__init__.py","schema_version":1,"sha256":"ac164c5f06ea4ca831d6dc5c0bccc783083ea418dd1ffb500ff99a3ffc1206ca","size_bytes":155},{"path":"plugin_payload/hermes-workflows-approvals/dashboard/dist/index.js","schema_version":1,"sha256":"1d9ec2b5580fff8115bd1baa48a4673c443a329d8f7cb6fcbba2cf13e695281f","size_bytes":90830},{"path":"plugin_payload/hermes-workflows-approvals/dashboard/dist/style.css","schema_version":1,"sha256":"d69785262f38704ff233a59352755fbe77b9a5e0dbf8a5f2415a0f4ec943ac3a","size_bytes":19192},{"path":"plugin_payload/hermes-workflows-approvals/dashboard/manifest.json","schema_version":1,"sha256":"397be901686af9369ead9d925df452a700064f6547a2939902277615931d256e","size_bytes":399},{"path":"plugin_payload/hermes-workflows-approvals/dashboard/plugin_api.py","schema_version":1,"sha256":"a1990fed6595cc89906aba3f7f3e82716c922e1a86e6305d8af87773a61fc64c","size_bytes":104986},{"path":"plugin_payload/hermes-workflows-approvals/plugin.yaml","schema_version":1,"sha256":"54ef3dc362939103f654d424b41c502b93145d62dfa771b9a00f187afab39dd0","size_bytes":416}],"owner_id":"hermes-workflows","package_name":"hermes-workflows","package_version":"0.0.1rc1","payload_root":"plugin_payload","schema_version":1} \ No newline at end of file diff --git a/tests/test_package_resource_contract.py b/tests/test_package_resource_contract.py index f63f18e..d7c83ee 100644 --- a/tests/test_package_resource_contract.py +++ b/tests/test_package_resource_contract.py @@ -4,7 +4,7 @@ import sys import zipfile from dataclasses import FrozenInstanceError -from importlib import metadata +from importlib import metadata, resources from pathlib import Path import pytest @@ -23,6 +23,14 @@ REPO_ROOT = Path(__file__).resolve().parents[1] MANIFEST_NAME = "plugin_payload_manifest.v1.json" +PAYLOAD_PATHS = ( + "plugin_payload/hermes-workflows-approvals/__init__.py", + "plugin_payload/hermes-workflows-approvals/dashboard/dist/index.js", + "plugin_payload/hermes-workflows-approvals/dashboard/dist/style.css", + "plugin_payload/hermes-workflows-approvals/dashboard/manifest.json", + "plugin_payload/hermes-workflows-approvals/dashboard/plugin_api.py", + "plugin_payload/hermes-workflows-approvals/plugin.yaml", +) def _file(path="plugin_payload/example.txt", sha256="a" * 64, size_bytes=1): @@ -69,16 +77,26 @@ def to_dict(self): } -def test_foundation_manifest_is_frozen_empty_and_canonical(): +def _packaged_bytes(relative: str) -> bytes: + resource = resources.files("hermes_workflows") + for part in relative.split("/"): + resource = resource.joinpath(part) + return resource.read_bytes() + + +def test_foundation_manifest_is_frozen_nonempty_and_canonical(): manifest = foundation_manifest() - assert manifest == _manifest() - assert manifest.files == () + assert tuple(entry.path for entry in manifest.files) == PAYLOAD_PATHS + for entry in manifest.files: + data = _packaged_bytes(entry.path) + assert entry.sha256 == hashlib.sha256(data).hexdigest() + assert entry.size_bytes == len(data) with pytest.raises(FrozenInstanceError): manifest.owner_id = "other" expected = { - "files": [], + "files": [entry.to_dict() for entry in manifest.files], "owner_id": "hermes-workflows", "package_name": "hermes-workflows", "package_version": installed_package_version(), @@ -123,11 +141,19 @@ def test_manifest_resource_is_present_and_canonical_in_built_wheel(tmp_path): assert len(wheels) == 1 member = "hermes_workflows/" + MANIFEST_NAME + payload_members = tuple("hermes_workflows/" + path for path in PAYLOAD_PATHS) with zipfile.ZipFile(wheels[0]) as archive: assert member in archive.namelist() resource_text = archive.read(member).decode("utf-8") + manifest = manifest_from_json(resource_text) + assert tuple(entry.path for entry in manifest.files) == PAYLOAD_PATHS + for entry, payload_member in zip(manifest.files, payload_members): + assert payload_member in archive.namelist() + data = archive.read(payload_member) + assert entry.sha256 == hashlib.sha256(data).hexdigest() + assert entry.size_bytes == len(data) - assert resource_text == canonical_manifest_json(foundation_manifest()) + assert resource_text == canonical_manifest_json(manifest) def test_canonical_json_round_trips_and_rejects_unknown_fields(): @@ -281,10 +307,11 @@ def test_all_schema_versions_must_equal_one(): _manifest(schema_version=2) -def test_empty_payload_performs_no_filesystem_write(tmp_path): +def test_packaged_payload_writes_validated_bytes_to_a_new_destination(tmp_path): destination = tmp_path / "must-not-exist" written = write_package_payload(foundation_manifest(), destination) - assert written == () - assert not destination.exists() + assert tuple(path.relative_to(destination).as_posix() for path in written) == PAYLOAD_PATHS + for path, relative in zip(written, PAYLOAD_PATHS): + assert path.read_bytes() == _packaged_bytes(relative) diff --git a/tests/test_package_resources.py b/tests/test_package_resources.py new file mode 100644 index 0000000..dd9a0df --- /dev/null +++ b/tests/test_package_resources.py @@ -0,0 +1,140 @@ +from __future__ import annotations + +import json +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +from hermes_workflows.package_resources import ( + PackageResourceManifestV1, + foundation_manifest, + write_package_payload, +) + + +REPO_ROOT = Path(__file__).resolve().parents[1] + + +def _manifest_with(*, files): + manifest = foundation_manifest() + return PackageResourceManifestV1( + schema_version=manifest.schema_version, + owner_id=manifest.owner_id, + package_name=manifest.package_name, + package_version=manifest.package_version, + payload_root=manifest.payload_root, + files=files, + ) + + +@pytest.mark.parametrize("change", ["missing", "extra", "hash", "size"]) +def test_copy_refuses_manifest_drift_without_touching_destination(tmp_path: Path, change: str): + manifest = foundation_manifest() + files = list(manifest.files) + if change == "missing": + files.pop() + elif change == "extra": + last = files[-1] + files.append(last.__class__(1, "plugin_payload/z-extra.txt", "0" * 64, 0)) + elif change == "hash": + first = files[0] + files[0] = first.__class__(1, first.path, "0" * 64, first.size_bytes) + else: + first = files[0] + files[0] = first.__class__(1, first.path, first.sha256, first.size_bytes + 1) + hostile = _manifest_with(files=tuple(files)) + destination = tmp_path / "destination" + + with pytest.raises(ValueError, match="packaged payload manifest"): + write_package_payload(hostile, destination) + + assert not destination.exists() + + +def test_copy_refuses_user_owned_destination_without_deleting_files(tmp_path: Path): + destination = tmp_path / "destination" + destination.mkdir() + user_file = destination / "mine.txt" + user_file.write_bytes(b"keep me") + + with pytest.raises(FileExistsError, match="destination"): + write_package_payload(foundation_manifest(), destination) + + assert user_file.read_bytes() == b"keep me" + + +def test_copy_refuses_symlinked_destination_ancestor_without_writing_through_it(tmp_path: Path): + outside = tmp_path / "outside" + outside.mkdir() + alias = tmp_path / "alias" + alias.symlink_to(outside, target_is_directory=True) + + with pytest.raises(ValueError, match="symlink"): + write_package_payload(foundation_manifest(), alias / "destination") + + assert tuple(outside.iterdir()) == () + + +def test_clean_installed_wheel_reads_validates_copies_and_installs_without_source_checkout(tmp_path: Path): + outdir = tmp_path / "dist" + subprocess.run( + [sys.executable, "-m", "build", "--outdir", str(outdir)], + cwd=REPO_ROOT, + check=True, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + wheel = next(outdir.glob("*.whl")) + venv = tmp_path / "venv" + subprocess.run([sys.executable, "-m", "venv", str(venv)], check=True) + python = venv / ("Scripts/python.exe" if os.name == "nt" else "bin/python") + subprocess.run( + [str(python), "-m", "pip", "install", "--no-deps", str(wheel)], + check=True, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + scratch = tmp_path / "outside-source-checkout" + scratch.mkdir() + script = """ +import json +from pathlib import Path +import hermes_workflows +from hermes_workflows.package_resources import foundation_manifest, write_package_payload +from hermes_workflows.plugin_install import install_plugin, inspect_payload + +root = Path.cwd() +manifest = foundation_manifest() +written = write_package_payload(manifest, root / "copied") +descriptor = inspect_payload() +report = install_plugin(root / "profile") +print(json.dumps({ + "package_file": hermes_workflows.__file__, + "manifest_paths": [entry.path for entry in manifest.files], + "written": [path.relative_to(root / "copied").as_posix() for path in written], + "inspected": list(descriptor.files), + "installed": list(report.files), +}, sort_keys=True)) +""" + env = os.environ.copy() + env.pop("PYTHONPATH", None) + completed = subprocess.run( + [str(python), "-c", script], + cwd=scratch, + env=env, + check=True, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + receipt = json.loads(completed.stdout) + + assert str(REPO_ROOT) not in receipt["package_file"] + assert receipt["manifest_paths"] == receipt["written"] + assert [path.split("hermes-workflows-approvals/", 1)[1] for path in receipt["manifest_paths"]] == receipt["inspected"] + assert receipt["inspected"] == receipt["installed"] diff --git a/tests/test_plugin_install.py b/tests/test_plugin_install.py index 6cbe9bc..aa1737d 100644 --- a/tests/test_plugin_install.py +++ b/tests/test_plugin_install.py @@ -11,6 +11,7 @@ import pytest from hermes_workflows import plugin_install +from hermes_workflows.package_resources import PackageResourceManifestV1, foundation_manifest REPO_ROOT = Path(__file__).resolve().parents[1] @@ -440,6 +441,27 @@ def test_canonical_payload_has_one_version_and_all_dashboard_surfaces(): assert (PAYLOAD_ROOT / "dashboard" / "plugin_api.py").read_text(encoding="utf-8").find("router = APIRouter()") >= 0 +def test_default_install_refuses_package_manifest_byte_mismatch_before_profile_mutation(tmp_path: Path, monkeypatch): + manifest = foundation_manifest() + first = manifest.files[0] + mismatched = first.__class__(first.schema_version, first.path, "0" * 64, first.size_bytes) + hostile = PackageResourceManifestV1( + schema_version=manifest.schema_version, + owner_id=manifest.owner_id, + package_name=manifest.package_name, + package_version=manifest.package_version, + payload_root=manifest.payload_root, + files=(mismatched,) + manifest.files[1:], + ) + monkeypatch.setattr(plugin_install.package_resources, "foundation_manifest", lambda: hostile) + profile = tmp_path / "profile" + + with pytest.raises(plugin_install.PayloadValidationError, match="manifest"): + plugin_install.install_plugin(profile) + + assert not profile.exists() + + def test_install_is_profile_scoped_atomic_enabled_and_reports_reload_contract(tmp_path: Path): profile = tmp_path / "fresh-profile" From 00e130e182eb484142da059c782709a9ad7a0c8b Mon Sep 17 00:00:00 2001 From: Skylar Payne <4830598+skylarbpayne@users.noreply.github.com> Date: Wed, 15 Jul 2026 11:47:30 -0700 Subject: [PATCH 2/3] fix: bind package payload writes to directory descriptors --- src/hermes_workflows/package_resources.py | 149 ++++++++++++++++++++-- tests/test_package_resources.py | 59 +++++++++ 2 files changed, 200 insertions(+), 8 deletions(-) diff --git a/src/hermes_workflows/package_resources.py b/src/hermes_workflows/package_resources.py index a349f3e..4d10f18 100644 --- a/src/hermes_workflows/package_resources.py +++ b/src/hermes_workflows/package_resources.py @@ -2,8 +2,8 @@ import hashlib import json +import os import re -import shutil import stat from dataclasses import dataclass from importlib import metadata, resources @@ -254,6 +254,95 @@ def _validate_new_destination(root: Path) -> None: raise ValueError(f"package payload destination ancestor is not a directory: {current}") +def _directory_open_flags() -> int: + required_flags = ("O_DIRECTORY", "O_NOFOLLOW") + if any(not hasattr(os, name) for name in required_flags): + raise OSError("package payload writes require no-follow directory descriptors") + required_dir_fd = (os.open, os.mkdir, os.stat, os.unlink, os.rmdir) + if any(function not in os.supports_dir_fd for function in required_dir_fd): + raise OSError("package payload writes require descriptor-relative filesystem operations") + if os.stat not in os.supports_follow_symlinks: + raise OSError("package payload writes require no-follow descriptor-relative stat") + return os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW | getattr(os, "O_CLOEXEC", 0) + + +def _open_directory_path(path: Path) -> int: + flags = _directory_open_flags() + descriptor = os.open(path.anchor, flags) + current = Path(path.anchor) + try: + for component in path.parts[1:]: + current = current / component + try: + child = os.open(component, flags, dir_fd=descriptor) + except OSError as exc: + try: + mode = os.stat(component, dir_fd=descriptor, follow_symlinks=False).st_mode + except OSError: + raise ValueError(f"package payload destination parent changed during validation: {current}") from exc + if stat.S_ISLNK(mode): + raise ValueError(f"package payload destination contains symlink ancestor: {current}") from exc + if not stat.S_ISDIR(mode): + raise ValueError(f"package payload destination ancestor is not a directory: {current}") from exc + raise ValueError(f"package payload destination parent changed during validation: {current}") from exc + os.close(descriptor) + descriptor = child + return descriptor + except Exception: + os.close(descriptor) + raise + + +def _same_file_identity(left: os.stat_result, right: os.stat_result) -> bool: + return left.st_dev == right.st_dev and left.st_ino == right.st_ino + + +def _entry_matches_stat(parent_fd: int, name: str, expected: os.stat_result) -> bool: + try: + actual = os.stat(name, dir_fd=parent_fd, follow_symlinks=False) + except OSError: + return False + return _same_file_identity(actual, expected) + + +def _entry_matches_fd(parent_fd: int, name: str, descriptor: int) -> bool: + try: + actual = os.stat(name, dir_fd=parent_fd, follow_symlinks=False) + expected = os.fstat(descriptor) + except OSError: + return False + return stat.S_ISDIR(actual.st_mode) and _same_file_identity(actual, expected) + + +def _path_matches_fd(path: Path, descriptor: int) -> bool: + try: + current = _open_directory_path(path) + except (OSError, ValueError): + return False + try: + return _same_file_identity(os.fstat(current), os.fstat(descriptor)) + finally: + os.close(current) + + +def _remove_created_entries( + files: list[Tuple[int, str, os.stat_result]], + directories: list[Tuple[int, str, os.stat_result]], +) -> None: + for parent_fd, name, expected in reversed(files): + if _entry_matches_stat(parent_fd, name, expected): + try: + os.unlink(name, dir_fd=parent_fd) + except OSError: + pass + for parent_fd, name, expected in reversed(directories): + if _entry_matches_stat(parent_fd, name, expected): + try: + os.rmdir(name, dir_fd=parent_fd) + except OSError: + pass + + def write_package_payload( manifest: PackageResourceManifestV1, destination: PathLike, @@ -267,19 +356,63 @@ def write_package_payload( payload = _validated_payload_bytes(packaged) root = Path(destination) _validate_new_destination(root) - if root.exists() or root.is_symlink(): + if not root.name: raise FileExistsError(f"refusing to replace existing destination: {root}") - root.mkdir() + parent_fd = _open_directory_path(root.parent) + root_fd = None + directory_fds = {} + created_files = [] + created_directories = [] written = [] try: + try: + os.mkdir(root.name, dir_fd=parent_fd) + except FileExistsError as exc: + raise FileExistsError(f"refusing to replace existing destination: {root}") from exc + root_stat = os.stat(root.name, dir_fd=parent_fd, follow_symlinks=False) + created_directories.append((parent_fd, root.name, root_stat)) + root_fd = os.open(root.name, _directory_open_flags(), dir_fd=parent_fd) + if not _entry_matches_fd(parent_fd, root.name, root_fd): + raise ValueError("package payload destination changed during creation") + directory_fds[()] = root_fd + for entry, data in payload: - path = root / entry.path - path.parent.mkdir(parents=True, exist_ok=True) - with path.open("xb") as handle: + parts = PurePosixPath(entry.path).parts + parent_parts = () + for component in parts[:-1]: + child_parts = parent_parts + (component,) + if child_parts not in directory_fds: + component_parent_fd = directory_fds[parent_parts] + os.mkdir(component, dir_fd=component_parent_fd) + component_stat = os.stat(component, dir_fd=component_parent_fd, follow_symlinks=False) + created_directories.append((component_parent_fd, component, component_stat)) + child_fd = os.open(component, _directory_open_flags(), dir_fd=component_parent_fd) + if not _entry_matches_fd(component_parent_fd, component, child_fd): + os.close(child_fd) + raise ValueError("package payload destination directory changed during creation") + directory_fds[child_parts] = child_fd + parent_parts = child_parts + + file_parent_fd = directory_fds[parent_parts] + file_flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | os.O_NOFOLLOW | getattr(os, "O_CLOEXEC", 0) + file_fd = os.open(parts[-1], file_flags, 0o666, dir_fd=file_parent_fd) + file_stat = os.fstat(file_fd) + created_files.append((file_parent_fd, parts[-1], file_stat)) + with os.fdopen(file_fd, "wb") as handle: handle.write(data) - written.append(path) + written.append(root / entry.path) + + if not _path_matches_fd(root.parent, parent_fd) or not _entry_matches_fd(parent_fd, root.name, root_fd): + raise ValueError("package payload destination changed before completion") except Exception: - shutil.rmtree(root) + _remove_created_entries(created_files, created_directories) raise + finally: + for parts, descriptor in reversed(tuple(directory_fds.items())): + if parts: + os.close(descriptor) + if root_fd is not None: + os.close(root_fd) + os.close(parent_fd) return tuple(written) diff --git a/tests/test_package_resources.py b/tests/test_package_resources.py index dd9a0df..98a9014 100644 --- a/tests/test_package_resources.py +++ b/tests/test_package_resources.py @@ -8,6 +8,7 @@ import pytest +import hermes_workflows.package_resources as package_resources from hermes_workflows.package_resources import ( PackageResourceManifestV1, foundation_manifest, @@ -78,6 +79,64 @@ def test_copy_refuses_symlinked_destination_ancestor_without_writing_through_it( assert tuple(outside.iterdir()) == () +def test_copy_refuses_destination_ancestor_swapped_after_validation( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + validated_parent = tmp_path / "validated-parent" + validated_parent.mkdir() + moved_parent = tmp_path / "moved-parent" + outside = tmp_path / "outside" + outside.mkdir() + user_file = outside / "mine.txt" + user_file.write_bytes(b"keep me") + destination = validated_parent / "destination" + real_validate = package_resources._validate_new_destination + + def validate_then_swap(root: Path) -> None: + real_validate(root) + validated_parent.rename(moved_parent) + validated_parent.symlink_to(outside, target_is_directory=True) + + monkeypatch.setattr(package_resources, "_validate_new_destination", validate_then_swap) + + with pytest.raises(ValueError, match="destination"): + write_package_payload(foundation_manifest(), destination) + + assert user_file.read_bytes() == b"keep me" + assert not (outside / "destination").exists() + assert tuple(moved_parent.iterdir()) == () + + +def test_copy_cleans_descriptor_relative_when_parent_is_swapped_before_return( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +): + bound_parent = tmp_path / "bound-parent" + bound_parent.mkdir() + moved_parent = tmp_path / "moved-parent" + outside = tmp_path / "outside" + outside.mkdir() + user_file = outside / "mine.txt" + user_file.write_bytes(b"keep me") + destination = bound_parent / "destination" + real_path_matches = package_resources._path_matches_fd + + def swap_then_match(path: Path, descriptor: int) -> bool: + bound_parent.rename(moved_parent) + bound_parent.symlink_to(outside, target_is_directory=True) + return real_path_matches(path, descriptor) + + monkeypatch.setattr(package_resources, "_path_matches_fd", swap_then_match) + + with pytest.raises(ValueError, match="destination"): + write_package_payload(foundation_manifest(), destination) + + assert user_file.read_bytes() == b"keep me" + assert not (outside / "destination").exists() + assert tuple(moved_parent.iterdir()) == () + + def test_clean_installed_wheel_reads_validates_copies_and_installs_without_source_checkout(tmp_path: Path): outdir = tmp_path / "dist" subprocess.run( From d51c69b3db4e4b2ca17f5d1072dd58e8b910228b Mon Sep 17 00:00:00 2001 From: Skylar Payne <4830598+skylarbpayne@users.noreply.github.com> Date: Wed, 15 Jul 2026 12:38:57 -0700 Subject: [PATCH 3/3] fix: exclude private presentation docs from sdist --- pyproject.toml | 1 + tests/test_package_resources.py | 54 +++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/pyproject.toml b/pyproject.toml index 4578d86..777d31f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -45,6 +45,7 @@ packages = ["src/hermes_workflows"] "src/hermes_workflows/plugin_payload/hermes-workflows-approvals/dashboard/dist/style.css" = "hermes_workflows/plugin_payload/hermes-workflows-approvals/dashboard/dist/style.css" [tool.hatch.build.targets.sdist] +exclude = ["docs/presentation/2026-07-02/**"] artifacts = [ "src/hermes_workflows/plugin_payload/hermes-workflows-approvals/dashboard/dist/index.js", "src/hermes_workflows/plugin_payload/hermes-workflows-approvals/dashboard/dist/style.css", diff --git a/tests/test_package_resources.py b/tests/test_package_resources.py index 98a9014..cf743cb 100644 --- a/tests/test_package_resources.py +++ b/tests/test_package_resources.py @@ -2,8 +2,11 @@ import json import os +import re +import shutil import subprocess import sys +import tarfile from pathlib import Path import pytest @@ -137,6 +140,57 @@ def swap_then_match(path: Path, descriptor: int) -> bool: assert tuple(moved_parent.iterdir()) == () +def test_clean_sdist_contains_no_private_absolute_user_paths(tmp_path: Path): + source_root = tmp_path / "source" + tracked = subprocess.run( + ["git", "ls-files", "-z"], + cwd=REPO_ROOT, + check=True, + stdout=subprocess.PIPE, + ).stdout.split(b"\0") + for encoded_relative in tracked: + if not encoded_relative: + continue + relative = Path(os.fsdecode(encoded_relative)) + source = REPO_ROOT / relative + destination = source_root / relative + destination.parent.mkdir(parents=True, exist_ok=True) + if source.is_symlink(): + destination.symlink_to(os.readlink(source)) + else: + shutil.copyfile(source, destination) + + outdir = tmp_path / "dist" + subprocess.run( + [sys.executable, "-m", "build", "--sdist", "--outdir", str(outdir)], + cwd=source_root, + check=True, + text=True, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + sdist = next(outdir.glob("*.tar.gz")) + account = b"skylarpayne" + unix_markers = (b"/" + b"Users" + b"/" + account, b"/" + b"home" + b"/" + account) + windows_user_path = re.compile( + rb"[A-Za-z]:[\\/](?:Users|Documents and Settings)[\\/]" + account + rb"(?:[\\/]|$)", + re.IGNORECASE, + ) + leaks = [] + + with tarfile.open(sdist, "r:gz") as archive: + for member in archive.getmembers(): + if not member.isfile(): + continue + extracted = archive.extractfile(member) + assert extracted is not None + contents = extracted.read() + if any(marker in contents for marker in unix_markers) or windows_user_path.search(contents): + leaks.append(member.name) + + assert leaks == [] + + def test_clean_installed_wheel_reads_validates_copies_and_installs_without_source_checkout(tmp_path: Path): outdir = tmp_path / "dist" subprocess.run(