mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-11-07 19:20:34 +00:00
fix(provisioner): keep POSIX hostPath values POSIX on Windows hosts (#7905)
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.
This commit is contained in:
parent
097fa6dca7
commit
e163b02bea
@ -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)
|
||||
|
||||
@ -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"
|
||||
|
||||
@ -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}")
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user