From 44b88151be40ae8c1dfd05572fec8af63d44ad86 Mon Sep 17 00:00:00 2001 From: Felix Shi <3283655504@qq.com> Date: Thu, 29 Oct 2026 22:08:06 +0800 Subject: [PATCH] fix(skills): capture package digests without POSIX-only dir_fd (#7583) * fix(skills): capture package digests without POSIX-only dir_fd `capture_package` rejected every non-POSIX host, so `SkillMutationRuntime.state` fell back to `"unsupported-" + uuid4().hex`. That digest differs on every call, so `repository.observe()` recorded a mutation for an untouched package: `mutation_seq` kept climbing and ABA detection could never match. Walk the tree with `scandir` + `lstat` where `dir_fd`/`O_NOFOLLOW` are missing, comparing the pre-open `lstat` against the opened descriptor before reading, and leave the POSIX descriptor walk unchanged. Windows 11 / Python 3.12: test_skill_asset_versions.py 5 failed -> 23 passed; reverting this file alone brings the 5 failures back. * docs(gateway): keep guidance below CI size budget --------- Co-authored-by: Felix Shi <204130665+shi-felix@users.noreply.github.com> Co-authored-by: Willem Jiang --- backend/app/gateway/AGENTS.md | 42 ++++--- .../deerflow/skills/mutations/assets.py | 109 +++++++++++++++++- backend/tests/test_skill_asset_versions.py | 2 +- 3 files changed, 131 insertions(+), 22 deletions(-) diff --git a/backend/app/gateway/AGENTS.md b/backend/app/gateway/AGENTS.md index 6fe70ebd7a..e71cd29f06 100644 --- a/backend/app/gateway/AGENTS.md +++ b/backend/app/gateway/AGENTS.md @@ -76,28 +76,32 @@ OIDC redirects retain existing endpoint query bytes (RFC 6749 section 3.1). Malformed IdP bodies raise `OIDCError` (502/`sso_failed`). `tests/test_oidc_auth.py` pins query, state-cookie, nonce and PKCE agreement. -Personal Access Tokens (`app.gateway.auth.pat`, `Authorization: Bearer dfp_...`) run as their owning user: an invalid Bearer is a hard 401 with no cookie fallback, which keeps `CSRFMiddleware`'s Bearer skip safe (origin checks still run). Scopes narrow within the allowlisted threads/runs/projects routes (including `POST /api/threads/{id}/move`); every other authenticated route 403s PAT callers (admin included). PAT management and `/change-password` require session auth; only SHA-256 digests are stored (`0017`). +PATs (`app.gateway.auth.pat`, `Authorization: Bearer dfp_...`) act as their owner. +Invalid Bearers return 401 without cookie fallback, keeping `CSRFMiddleware`'s +Bearer skip safe; origin checks still run. Scopes narrow allowlisted thread/run/ +project routes, including `POST /api/threads/{id}/move`; other authenticated +routes reject PATs with 403, even for admins. PAT management and `/change-password` +require session auth. Store only SHA-256 digests (`0017`). -Thread→project membership is written by thread creation (`POST /api/threads` with -a validated `project_id`), branch creation (the new row inherits the source -thread's project; an archived/deleted project degrades the branch to unassigned -instead of failing), and explicit moves (`POST /api/threads/{id}/move`); run -admission never modifies membership; it only pins the resolved project context -read-only into the run context. The server-reserved `deerflow_project_id` -metadata key is a read-only exposure of the `threads_meta.project_id` column and -is stripped from client writes. +Thread→project membership is written only by thread creation (`POST /api/threads` +with validated `project_id`), branch creation (inherit the source project; +archived/deleted projects become unassigned rather than failing), and explicit +`POST /api/threads/{id}/move`. Run admission pins resolved project context +read-only and never changes membership. Server-reserved `deerflow_project_id` +metadata exposes `threads_meta.project_id` read-only and is stripped from client writes. -Localhost persistence deliberately reads the direct request `Host` and ignores `Forwarded` / `X-Forwarded-Host`. Scheme and auth-origin reconstruction still consume forwarding headers. The bundled nginx sets `X-Forwarded-Proto`, but preserves an upstream HTTPS value and does not overwrite every forwarded header, so the outer trusted proxy must replace or strip client-supplied forwarding headers before traffic reaches DeerFlow. +Localhost persistence trusts direct `Host`, not `Forwarded`/`X-Forwarded-Host`. +Scheme/auth-origin reconstruction still uses forwarding headers. Nginx sets +`X-Forwarded-Proto` but preserves upstream HTTPS and some other forwarded headers; +the outer trusted proxy must replace/strip client copies before DeerFlow. -Standalone local LangGraph Studio is recognized only through the upstream -`Auth.types.StudioUser` principal type, never by its reusable identity string. -The type is resolved once at import; an older SDK without it degrades to normal -owner scoping. -For that principal's assistant reads/searches, `langgraph_auth.add_owner_filter` -selects genuine server-registered assistants plus assistants owned by Studio; -all other resources remain owner-scoped. Assistant create/update handlers make -both `user_id` and `created_by=user` server-owned, because LangGraph gives -`created_by=system` privileged ownership semantics during run creation. The +Recognize standalone Studio only by upstream `Auth.types.StudioUser`, never its +reusable identity string. Resolve the type once at import; older SDKs without +it retain normal owner scoping. For its assistant reads/searches, +`langgraph_auth.add_owner_filter` admits genuine server-registered assistants +and Studio-owned rows; other resources stay owner-scoped. Assistant create/update +forces server-owned `user_id` and `created_by=user`: LangGraph treats +`created_by=system` as privileged run ownership. The custom application module in `langgraph_studio.py` is imported before the locked in-memory runtime lifespan. At that pre-runtime boundary it derives genuine system assistant IDs from the CLI-provided graph registry, removes diff --git a/backend/packages/harness/deerflow/skills/mutations/assets.py b/backend/packages/harness/deerflow/skills/mutations/assets.py index e01ee04ed5..caeebb591a 100644 --- a/backend/packages/harness/deerflow/skills/mutations/assets.py +++ b/backend/packages/harness/deerflow/skills/mutations/assets.py @@ -1,4 +1,11 @@ -"""Bounded byte snapshots for managed custom packages (POSIX, no links).""" +"""Bounded byte snapshots for managed custom packages (no links). + +POSIX walks the tree with ``dir_fd`` descriptors so every path component is +anchored. Hosts without ``dir_fd``/``O_NOFOLLOW`` (Windows) use +:func:`_capture_package_portable`, which re-checks each component with ``lstat`` +before use and compares the pre-open and post-open ``stat`` of every file, so a +swap during the walk is still rejected instead of followed. +""" from __future__ import annotations @@ -47,7 +54,7 @@ class PackageSnapshot: def capture_package(root: Path) -> PackageSnapshot: """Caller owns mutation guard. Never read outside the opened package tree.""" if os.name != "posix": - raise ValueError("UNSUPPORTED_ASSET") + return _capture_package_portable(root) files: list[PackageFile] = [] total = 0 entries = 0 @@ -146,3 +153,101 @@ def open_directory_chain(path: Path) -> int: except BaseException: os.close(fd) raise + + +def _resolve_unlinked_ancestors(path: Path) -> Path: + """Return *path* after rejecting a link in any component above it. + + The non-POSIX twin of :func:`open_directory_chain`: there is no ``dir_fd`` + to anchor the walk, so each component is ``lstat``-ed in order and a link + stops the walk instead of being traversed. + """ + path = path.absolute() + if ".." in path.parts or len(path.parts) > 128 or len(os.fsencode(path)) > 4096: + raise ValueError("UNSUPPORTED_ASSET") + current = Path(path.anchor) + for part in path.parts[1:]: + current = current / part + info = os.lstat(current) + if stat.S_ISLNK(info.st_mode) or not stat.S_ISDIR(info.st_mode): + raise ValueError("UNSUPPORTED_ASSET") + return path + + +def _capture_package_portable(root: Path) -> PackageSnapshot: + """Capture a package where ``dir_fd``/``O_NOFOLLOW`` are unavailable. + + ``os.lstat`` runs before every open and the opened descriptor is compared + against that ``lstat`` afterwards, so a component replaced between the two + calls raises ``REVISION_CONFLICT`` instead of publishing foreign bytes. + Windows keeps ``st_dev``/``st_ino`` for NTFS volumes, which makes that + comparison meaningful there; the executable bit stays unset because the + platform has no mode bits, and that is consistent within a host. + """ + files: list[PackageFile] = [] + total = 0 + entries = 0 + open_flags = os.O_RDONLY | getattr(os, "O_BINARY", 0) | getattr(os, "O_NOINHERIT", 0) + + def walk(directory: Path, prefix: str = "", depth: int = 0) -> None: + nonlocal total, entries + if depth > MAX_DEPTH: + raise ValueError("QUOTA_EXCEEDED") + names = [] + with os.scandir(directory) as iterator: + for entry in iterator: + entries += 1 + if entries > MAX_FILES * 2: + raise ValueError("QUOTA_EXCEEDED") + names.append(entry.name) + for name in sorted(names): + if "\\" in name or any(ord(c) < 32 for c in name): + raise ValueError("UNSUPPORTED_ASSET") + path = f"{prefix}/{name}" if prefix else name + if len(path.encode("utf-8")) > 1024: + raise ValueError("QUOTA_EXCEEDED") + target = directory / name + info = os.lstat(target) + if stat.S_ISLNK(info.st_mode): + raise ValueError("UNSUPPORTED_ASSET") + if stat.S_ISDIR(info.st_mode): + walk(target, path, depth + 1) + continue + if not stat.S_ISREG(info.st_mode): + raise ValueError("UNSUPPORTED_ASSET") + if len(files) >= MAX_FILES: + raise ValueError("QUOTA_EXCEEDED") + limit = min(MAX_PACKAGE_BYTES - total, MAX_MAIN_BYTES if path == "SKILL.md" else MAX_PACKAGE_BYTES) + if info.st_size > limit: + raise ValueError("QUOTA_EXCEEDED") + fd = os.open(target, open_flags) + try: + opened = os.fstat(fd) + if (info.st_dev, info.st_ino, info.st_mode) != (opened.st_dev, opened.st_ino, opened.st_mode): + raise ValueError("REVISION_CONFLICT") + data = bytearray() + while True: + chunk = os.read(fd, min(65536, limit + 1 - len(data))) + if not chunk: + break + data.extend(chunk) + if len(data) > limit: + raise ValueError("QUOTA_EXCEEDED") + after = os.fstat(fd) + if (opened.st_size, opened.st_mtime_ns, opened.st_ctime_ns) != (after.st_size, after.st_mtime_ns, after.st_ctime_ns): + raise ValueError("REVISION_CONFLICT") + total += len(data) + files.append(PackageFile(path, bytes(data), bool(opened.st_mode & 0o111))) + finally: + os.close(fd) + + try: + walk(_resolve_unlinked_ancestors(root)) + except OSError as exc: + raise ValueError("UNSUPPORTED_ASSET") from exc + result = PackageSnapshot(tuple(sorted(files, key=lambda item: item.path))) + try: + result.main_content + except (StopIteration, UnicodeError) as exc: + raise ValueError("UNSUPPORTED_ASSET") from exc + return result diff --git a/backend/tests/test_skill_asset_versions.py b/backend/tests/test_skill_asset_versions.py index 264d98a4a0..ce6a4a65c3 100644 --- a/backend/tests/test_skill_asset_versions.py +++ b/backend/tests/test_skill_asset_versions.py @@ -24,7 +24,7 @@ def assets(tmp_path, monkeypatch): storage = UserScopedSkillStorage("owner", host_path=str(tmp_path / "skills")) root = storage.get_custom_skill_dir("example") root.mkdir(parents=True) - (root / "SKILL.md").write_text(CONTENT, encoding="utf-8") + (root / "SKILL.md").write_text(CONTENT, encoding="utf-8", newline="") engine = create_engine(f"sqlite:///{tmp_path / 'mutations.db'}") Base.metadata.create_all(engine) sessions = sessionmaker(engine, expire_on_commit=False)