From e163b02bea0f6681f90e9c96c55ba3fdfb488b01 Mon Sep 17 00:00:00 2001 From: hataa <79907651+hata33@users.noreply.github.com> Date: Fri, 6 Nov 2026 11:19:13 +0800 Subject: [PATCH] fix(provisioner): keep POSIX hostPath values POSIX on Windows hosts (#7905) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit join_host_path joined POSIX-styled bases with the running OS's native path module, so a provisioner running on Windows emitted backslash hostPath values into pod specs — paths a Linux kubelet cannot resolve. The same leak existed in _host_base_dir_for_extra_mounts, _is_path_under_base, extra-mount validation, and PVC subPath computation, all of which used os.path on paths that are POSIX by contract. Route every non-Windows-styled path through posixpath and keep the PureWindowsPath branch only for genuinely Windows bases. Also re-spell the memory guide probe layout with forward slashes so backend/tests/test_memory_documentation.py compares against the documented layout instead of the test host's local separator. --- backend/tests/test_memory_documentation.py | 2 +- backend/tests/test_provisioner_pvc_volumes.py | 85 +++++++++++++++++-- docker/provisioner/app.py | 73 ++++++++++++---- 3 files changed, 135 insertions(+), 25 deletions(-) diff --git a/backend/tests/test_memory_documentation.py b/backend/tests/test_memory_documentation.py index 15d97479f2..82c7ef2bf3 100644 --- a/backend/tests/test_memory_documentation.py +++ b/backend/tests/test_memory_documentation.py @@ -72,7 +72,7 @@ def probe_layout() -> tuple[str, str]: """The per-user manifest and per-agent facts paths, as the runtime resolves them.""" memory_path = memory_file_path(DeerMemConfig(storage_path=PROBE_ROOT), user_id=PROBE_USER_ID) facts_path = agent_facts_directory(memory_path, PROBE_AGENT_NAME) - return str(memory_path.relative_to(PROBE_ROOT)), str(facts_path.relative_to(PROBE_ROOT)) + return memory_path.relative_to(PROBE_ROOT).as_posix(), facts_path.relative_to(PROBE_ROOT).as_posix() @pytest.mark.parametrize("path", MEMORY_GUIDES, ids=str) diff --git a/backend/tests/test_provisioner_pvc_volumes.py b/backend/tests/test_provisioner_pvc_volumes.py index 0a84eda413..c07f16e7e1 100644 --- a/backend/tests/test_provisioner_pvc_volumes.py +++ b/backend/tests/test_provisioner_pvc_volumes.py @@ -1,7 +1,6 @@ """Regression tests for provisioner three-way skills + PVC volume support.""" import pytest -from _host_path_helpers import posix_path def _thread_skill_mounts( @@ -142,7 +141,7 @@ class TestBuildVolumes: pub = volumes[0] assert pub.name == "skills-public" assert pub.host_path is not None - assert posix_path(pub.host_path.path).endswith("/skills_view/public") + assert pub.host_path.path == "/.deer-flow/skills_view/public" assert pub.host_path.type == "Directory" assert pub.persistent_volume_claim is None @@ -153,7 +152,7 @@ class TestBuildVolumes: custom = volumes[1] assert custom.name == "skills-custom" assert custom.host_path is not None - assert "users/user-7/skills_view/custom" in posix_path(custom.host_path.path) + assert custom.host_path.path == "/.deer-flow/users/user-7/skills_view/custom" assert custom.host_path.type == "Directory" def test_hostpath_skills_legacy_volume(self, provisioner_module): @@ -163,7 +162,7 @@ class TestBuildVolumes: legacy = volumes[2] assert legacy.name == "skills-legacy" assert legacy.host_path is not None - assert "users/default/skills_view/legacy" in posix_path(legacy.host_path.path) + assert legacy.host_path.path == "/.deer-flow/users/default/skills_view/legacy" assert legacy.host_path.type == "Directory" def test_hostpath_skills_projection_volumes_are_stable(self, provisioner_module): @@ -182,9 +181,8 @@ class TestBuildVolumes: provisioner_module.USERDATA_PVC_NAME = "" volumes = provisioner_module._build_volumes("my-thread-42") userdata_vol = volumes[-1] - path = posix_path(userdata_vol.host_path.path) - assert "my-thread-42" in path - assert path.endswith("user-data") + path = userdata_vol.host_path.path + assert path == "/.deer-flow/threads/my-thread-42/user-data" assert userdata_vol.host_path.type == "DirectoryOrCreate" # ── PVC mode (single-volume fallback) ────────────────────────────── @@ -249,7 +247,7 @@ class TestBuildVolumes: assert len(volumes) == 5 extra_vol = volumes[-1] assert extra_vol.name == "extra-0" - assert posix_path(extra_vol.host_path.path) == "/state/users/alice/integrations/lark-cli/config" + assert extra_vol.host_path.path == "/state/users/alice/integrations/lark-cli/config" assert extra_vol.host_path.type == "DirectoryOrCreate" def test_extra_mount_uses_userdata_pvc_when_configured(self, provisioner_module): @@ -1022,3 +1020,74 @@ class TestLarkCliBrokerSidecar: sidecar = next(c for c in pod.spec.containers if c.name == "lark-cli-broker") env = {e.name for e in (sidecar.env or [])} assert "DEERFLOW_LARK_BROKER_DENY_SUBCOMMANDS" not in env + + +# ── host-path style contract ──────────────────────────────────────────── + + +class TestHostPathStyle: + """join_host_path and friends keep POSIX bases POSIX on any host OS. + + CI runs on Linux, where the old native-path code already behaved; these + pins guard the Windows-host failure mode hostPath assertions could not + see when they normalized with posix_path(). + """ + + def test_join_host_path_keeps_posix_base_posix(self, provisioner_module): + assert provisioner_module.join_host_path("/state", "users", "x") == "/state/users/x" + assert provisioner_module.join_host_path("/state/", "users") == "/state/users" + assert provisioner_module.join_host_path("/state") == "/state" + + def test_join_host_path_keeps_windows_base_windows(self, provisioner_module): + assert provisioner_module.join_host_path(r"C:\state", "users") == r"C:\state\users" + assert provisioner_module.join_host_path(r"\\server\share", "users") == r"\\server\share\users" + + def test_norm_host_path_collapses_posix_segments(self, provisioner_module): + assert provisioner_module._norm_host_path("/state/./users") == "/state/users" + assert provisioner_module._norm_host_path("/state/users/../x") == "/state/x" + + def test_is_abs_host_path_judges_each_style_by_its_own_rules(self, provisioner_module): + assert provisioner_module._is_abs_host_path("/state/users") + assert not provisioner_module._is_abs_host_path("state/users") + assert provisioner_module._is_abs_host_path(r"C:\state") + + def test_is_path_under_base_posix(self, provisioner_module): + assert provisioner_module._is_path_under_base("/state/users/a", "/state") + assert not provisioner_module._is_path_under_base("/stateevil/a", "/state") + assert not provisioner_module._is_path_under_base("/state/users/a", "") + + def test_host_base_dir_from_threads_path(self, provisioner_module): + provisioner_module.DEER_FLOW_HOST_BASE_DIR = "" + provisioner_module.THREADS_HOST_PATH = "/state/threads" + assert provisioner_module._host_base_dir_for_extra_mounts() == "/state" + + +class TestWindowsHostSimulation: + """Simulate a Windows host by swapping the module's os.path to ntpath. + + The POSIX branches never touch os.path, so pod-spec hostPath values stay + forward-slashed even when the provisioner process runs on Windows; + before the native-path fix, os.path.normpath re-spelled /state/... with + backslashes exactly here. + """ + + @pytest.fixture() + def windows_host(self, monkeypatch, provisioner_module): + import ntpath + + monkeypatch.setattr(provisioner_module.os, "path", ntpath) + provisioner_module.DEER_FLOW_HOST_BASE_DIR = "/state" + return provisioner_module + + def test_join_host_path_ignores_host_os(self, windows_host): + assert windows_host.join_host_path("/state", "users", "x") == "/state/users/x" + + def test_host_base_dir_ignores_host_os(self, windows_host): + assert windows_host._host_base_dir_for_extra_mounts() == "/state" + + def test_extra_mount_validation_ignores_host_os(self, windows_host): + mounts = windows_host._validated_extra_mounts( + [windows_host.ExtraMount(host_path="/state/users/alice/config", container_path="/mnt/integrations/lark-cli/config", read_only=True)], + skills_container_path="/mnt/skills", + ) + assert mounts[0].host_path == "/state/users/alice/config" diff --git a/docker/provisioner/app.py b/docker/provisioner/app.py index 8c9593acaa..7111702741 100644 --- a/docker/provisioner/app.py +++ b/docker/provisioner/app.py @@ -31,13 +31,14 @@ Architecture (docker-compose-dev): from __future__ import annotations import logging +import ntpath import os import posixpath import re import secrets import time from contextlib import asynccontextmanager -from pathlib import PurePosixPath +from pathlib import PurePosixPath, PureWindowsPath import urllib3 from fastapi import FastAPI, HTTPException, Request, Response @@ -152,34 +153,70 @@ NODE_HOST = os.environ.get("NODE_HOST", "host.docker.internal") def join_host_path(base: str, *parts: str) -> str: - """Join host filesystem path segments while preserving native style.""" + """Join host filesystem path segments for a Kubernetes hostPath. + + Windows-styled bases keep Windows separators; every other base joins + POSIX-style regardless of the OS running the provisioner — a hostPath in + a pod spec is always interpreted by the (Linux) kubelet, and joining with + the native separator here would emit invalid backslash paths when the + provisioner itself runs on Windows. + """ if not parts: return base - if re.match(r"^[A-Za-z]:[\\/]", base) or base.startswith("\\\\") or "\\" in base: - from pathlib import PureWindowsPath - + if _is_windows_host_path(base): result = PureWindowsPath(base) for part in parts: result /= part return str(result) - from pathlib import Path - - result = Path(base) + result = PurePosixPath(base) for part in parts: result /= part return str(result) +def _is_windows_host_path(path: str) -> bool: + """Whether *path* is expressed with Windows host-path syntax.""" + return bool(re.match(r"^[A-Za-z]:[\\/]", path) or path.startswith("\\\\") or "\\" in path) + + +def _norm_host_path(path: str) -> str: + """Normalize a host path, keeping POSIX-style paths POSIX on any host OS. + + Kubernetes hostPath values are interpreted by a Linux kubelet, so a + ``/state/...`` root must not pick up the running OS's separators when the + provisioner itself happens to run on Windows. Windows-styled paths use + ``ntpath`` explicitly so their semantics do not flip with the host OS + either. + """ + if _is_windows_host_path(path): + return ntpath.normpath(path) + return posixpath.normpath(path) + + +def _is_abs_host_path(path: str) -> bool: + """Whether *path* is absolute under its own style. + + ``ntpath.isabs`` reports drive-less leading-slash paths like ``/state/x`` + as relative since Python 3.13, so POSIX-styled paths must be judged by + ``posixpath.isabs`` or a valid extra mount would be rejected on a Windows + host. + """ + if _is_windows_host_path(path): + return ntpath.isabs(path) + return posixpath.isabs(path) + + def _host_base_dir_for_extra_mounts() -> str: """Return the host-visible DeerFlow state root used for controlled mounts.""" if DEER_FLOW_HOST_BASE_DIR: - return os.path.normpath(DEER_FLOW_HOST_BASE_DIR) + return _norm_host_path(DEER_FLOW_HOST_BASE_DIR) - normalized_threads = os.path.normpath(THREADS_HOST_PATH) - if os.path.basename(normalized_threads) == "threads": - return os.path.dirname(normalized_threads) + normalized_threads = _norm_host_path(THREADS_HOST_PATH) + segments = ntpath if _is_windows_host_path(normalized_threads) else posixpath + if segments.basename(normalized_threads) == "threads": + return segments.dirname(normalized_threads) return "" @@ -187,8 +224,9 @@ def _is_path_under_base(path: str, base: str) -> bool: """Return whether *path* is inside *base* after normalization.""" if not base: return False + segments = ntpath if _is_windows_host_path(path) or _is_windows_host_path(base) else posixpath try: - return os.path.commonpath([os.path.normpath(path), os.path.normpath(base)]) == os.path.normpath(base) + return segments.commonpath([segments.normpath(path), segments.normpath(base)]) == segments.normpath(base) except ValueError: return False @@ -254,8 +292,8 @@ def _validated_extra_mounts( seen_container_paths: set[str] = set() validated: list[ExtraMount] = [] for mount in extra_mounts: - host_path = os.path.normpath(mount.host_path) - if not os.path.isabs(host_path): + host_path = _norm_host_path(mount.host_path) + if not _is_abs_host_path(host_path): raise HTTPException(status_code=400, detail=f"Extra mount host path must be absolute: {mount.host_path}") if not _is_path_under_base(host_path, host_base_dir): raise HTTPException(status_code=400, detail=f"Extra mount host path is outside DeerFlow state: {mount.host_path}") @@ -374,7 +412,10 @@ def _extra_mount_pvc_sub_path(host_path: str) -> str: if not _is_path_under_base(host_path, host_base_dir): raise HTTPException(status_code=400, detail=f"Extra mount host path is outside DeerFlow state: {host_path}") - rel_path = os.path.relpath(os.path.normpath(host_path), host_base_dir) + if _is_windows_host_path(host_path): + rel_path = ntpath.relpath(ntpath.normpath(host_path), host_base_dir) + else: + rel_path = posixpath.relpath(posixpath.normpath(host_path), host_base_dir) rel_parts = [part for part in rel_path.replace(os.sep, "/").split("/") if part and part != "."] if not rel_parts or any(part == ".." for part in rel_parts): raise HTTPException(status_code=400, detail=f"Invalid extra mount host path: {host_path}")